On Fri, 3 Aug 2018 at 11:24, Alistair Grant <[email protected]> wrote: > > Hi Guille, > > On Fri, Aug 03, 2018 at 10:34:22AM +0200, Guillermo Polito wrote: > > Hi, > > > > I don't know the windows situation at all, but I'm wondering. > > > > ([ File stdioDescriptorIsATTY not ] > > on: PrimitiveFailed > > do: [ :ex | "HACK kept for retrocompatibility" Smalltalk os isWin32 ]) > > ifTrue: [ ^ self createStdioFileFor: moniker ]. > > > > the first thing I would do, just trying to keep the same behaviour, is to > > invert the test... Something like > > > > Smalltalk os isWin32 ifTrue: [ > > [ File stdioDescriptorIsATTY not ] > > on: PrimitiveFailed > > do: [ :ex | false ]) > > ifTrue: [ ^ self createStdioFileFor: moniker ]. ] > > > > Like that, this would only penalise windows in the worst case. > > True. Actually I think it would be safe to remove it completely. Unix > platforms should always have stdio, and other platforms should act > appropriately. But more below... > > > > Then a first question arises in my head: why isWin32 and not > > isWindows? Is there a reason behind that? Wouldn't that prevent > > correct behaviour in windows 64 bits? > > You're right, it should be #isWindows. > > > > My next impression is that testing for "a TTY or a file or a pipe" > > seems buggy... We should put a more intention revealing selector like > > #isInvalidStdioHandle? > > A more high level test will allow us to do any check we want in the > > background, and it allows better to understand why we check for TTY or > > File or Pipe or la mar en coche :) > > I made the change so that Damien could test that it gives the behaviour > he expects, and did say that the selector will need to be renamed. > > > > What does #stdioHandles return to the image in the case of a > > non-console windows VM? I mean, when "it is not possible"? > > > From the docs, > > https://docs.microsoft.com/en-us/windows/console/getstdhandle says we > > may have an INVALID_HANDLE_VALUE or NULL in case it fails. > > Is the file plugin is masking the error? If so, thats why we need such > > strange workarounds... > > We'd still want the ability to test what type of device stdio is > connected to. But I agree the primitive should be doing the correct > thing. > > > > - If it is an invalid handle, why not just testing for > > "isInvalidHandle" or so? We can have that on the image side and then > > dispatch to the correct thing in the file plugin maybe? > > > - Maybe the #stdioHandles primitive should just fail in the case we > > don't have correct handles? that would allow us to do a much more > > elegant approach on the image side... > > I don't think the error handling has been changed in a long time, but > just looking quickly at the source code, I think you're correct that it > isn't returning the correct values. It should be returning nil for > those handles that aren't valid. > > I don't have my Windows VM handy at the moment, is anyone able to > quickly confirm? (Start pharo with Pharo.exe (not PharoConsole.exe) and > supply the result from: > > File stdioHandles > > > I've been asking lots of questions lately and not providing much input > (I was hoping to have File Attributes integrated by now, but MacOS file > encoding and my inability to get the VM to compile on a Mac VM have > frustrated me), so I'm more than happy to tidy up the stdio primitives. > > I'll need to check that it doesn't break anything on Squeak as well > (I don't think any of the changes discussed should cause problems).
Tracking issue: https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/274
