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). Cheers, Alistair
