Handle stdin on Windows using msvcrt functions

[pfmoore]

No description provided.

[codecov-io]

Current coverage is 93.12%

Merging #304 into master will decrease coverage by -0.09% as of d93685e

@@            master    #304   diff @@
======================================
  Files           20      20       
  Stmts         1725    1731     +6
  Branches       298     299     +1
  Methods          0       0       
======================================
+ Hit           1608    1612     +4
- Partial         31      32     +1
- Missed          86      87     +1

Review entire Coverage Diff as of d93685e

Powered by Codecov. Updated on successful CI builds.

[bitprophet]

I think I already have this in my #289 branch, but leaving this open in case I'm misremembering or if my implementation is missing something from here.

[pfmoore]

Cool - when's that likely to be released? Right now, invoke simply doesn't work on Windows unless I pin to an older version...

[bitprophet]

It's at the top of the priority list right now :)

[bitprophet]

OK I think my implementation is close enough to yours, given that I refactored where some of these bits live / abstracted them slightly. I'm working on the branch today but pushed its current state: https://github.com/pyinvoke/invoke/compare/rest-of-289

Will close this now but feel free to comment on #289 if you identify more problems with how I'm using msvcrt. BTW, I've got some of your other recently opened issues in my milestone too and will be checking on them before I merge. Chances are they'll also want to roll into #289 to limit the number of places we're having discussions...will see :)

[pfmoore]

Fantastic - I'll have a look at #289 for you, probably tomorrow. Sorry if I was being a bit pushy over this :-)

[pfmoore]

@bitprophet I'm not sure how to comment on that compare link you posted, so I'm going to post here.

Why do you need to use msvcrt.getch() in read_char? In my testing, input_.read(1) worked fine. Problems with getch:

• No echo
• Ctrl-C processing is suppressed, there's no way to break out
• Function keys return multi-byte sequences, one byte for each getch() call, and it's not clear how that interacts with kbhit.

You may get better results with getwche, but honestly I'd stick with read() unless you know there's a problem with it (I promise I'll let you know ::-))

I assume from the name, that read_byte must only be called with streams opened in binary mode? So not sys.stdin on Python 3? Otherwise you will get a (Unicode) character. And yet in handle_stdin you do self.write_stdin(self.encode(byte)) - which doesn't make sense as you don't encode bytes, you decode them - so it looks like you expect read_byte to return a Unicode character, not a byte.

Overall, it feels to me like your separation of byte and character is muddled. You may be able to get things working for now, but I'd strongly recommend reviewing the code and making a clean separation between bytes and characters, or you'll spend forever fighting {de,en}coding issues. I'd be willing to do such a review, if you want - but it's not really appropriate for this change right now. Just avoid msvcrt.getch and stick with read() and you should be OK for the moment.

[bitprophet]

That code is (aside from the reorganization) ported straight from Fabric 1, whose Windows support (like this project's) comes entirely from users like you. So, I have no idea why getch() was considered the right approach at the time :)

I'm totally still not grokking the gory details of Python 3's bytes vs strings nonsense, despite trying to understand it for years, which is why that's all very muddled. I'm not paid enough to get it working perfectly so my MO has been to get things working for the base case on 2 and 3 and to rely on you & others to raise flags when things seriously break (and then to rely on you providing tests to help prove when things work or don't).

Anyway let's take this to #289 from now so I'm not getting 2-3 times the thread notifications ;) thanks!!