debugger: leave output screen with single key press #674
Conversation
Introduce support code for non-buffered console input (single key press that doesn't accumulate input until ENTER is pressed). Support Windows (msvcrt) and POSIX supporting Unix platforms (select and termios). Fall back to the Python's input() routine for Webassembly and Emscripten. Avoid the accumulation of repeated prompts on platforms that support single key presses. Only the backwards compatible fallback on minor platforms involves a prompt, so users can tell when they face that situation. The approach in this commit automatically detects platforms, no config option is required. All depencencies are provided by Python core, there are no external dependencies. The implementation is considered suitable for future maintenance, extension, or adjustment for other platforms.
…bools Prefer a single enum over a set of individual booleans for the detection of the backend which implements single key getting. These code paths are strictly alternative and mutually exclusive. Use the catch-all "else" for the most portable Python input(3) call. TODO The "nbc" prefix may need adjustment later when renaming the class to better reflect that it's a single key getter.
Better reflect the purpose of the platform specific helper. Rename the class and its get_single_key() method which callers invoke explicitly. Discuss the purpose of the class and the context when it's used in the class' doc string for improved awareness. Could have been lost/missed in the comment.
Reflect that the main purpose is to read single keys. Non-buffered terminal mode is a byproduct, intermediate phase that's created and restored and only applies to part of the supported platforms. Have yet to come up with a better name, something that's not as clumsy as "keyread impl" is.
The motivation for the second getch() call was non-obvious. The magic values without any context made it worse. Point to the Python reference, phrase in own words what's happening (special keys require two calls, the first call's return value tells you when it's needed).
|
ACK on all your feedback items. Have quickly pushed 515ecc8..bc27f14 and will later look at lint and ruff, and test some more. Got ideas or preferences for the enum names? Drop the "impl" in the enum values? Keep _KEYREAD_GETCH et al for the available choices, use _keyread_impl for the variable that's dispatched on? |
|
Have addressed ruff. Don't understand lint. An ubuntu runner tries to import msvcrt although it's under sys.platform and should only apply to "win32"? What am I missing? If there were Windows installations (default setups, nothing customized) that don't have msvcrt then there would be a problem. If non-Windows platforms identify as win32 then there's another. :) Am currently assuming it's a false positive, or easy to fix when identified what triggered it. |
|
Pushed some fixes. How does this look to you? |
inducer
force-pushed
the
show-output-v2
branch
from
0d72282 to
99a3328
Compare
Put the NonBufferedConsole class and example calling code into a separate script, to check the implementation's portability on more platforms and outside the (more demanding) context of PuDB. Passed on Windows, Cygwin, WSL, and native Linux. Is assumed to work on any POSIX supporting Unix, too. Exclusively uses core modules, does not depend on 3rd party libraries. Worked with Python 3.9, 3.10, and 3.12. Is believed to not depend on a specific Python version. This is NOT suitable for upstream. It's a helper for evaluation of the approach. Not meant for inclusion in the PuDB project.
Apply feedback from review. Proper enums, strict alternative code paths. Late imports. Select(3) configuration shall never fail, catch-all case for the input() call.
|
Could you comment on your plan? I appreciate the thorough testing, but I also would like to move on from this. If you are OK with this, my preference would be to remove your |
|
Have pushed the test script this morning to run more tests, as I mentioned earlier ("will test more"). Have verified the operation of the approach on Windows and Cygwin again, while operation on Linux was known and Mac was reasonably assumed. Did not intend to confuse you after you spent so much time in the thorough review, which I appreciate very much. Have pushed the cleaned up result now in the afternoon, which you can pick up in my opinion. Have taken in all of your feedback. When you are happy you could merge --ff-only my v3 branch, or cherry-pick the single commit. Or tell if you want me to submit a PR for v3. Just don't take v2 which is dirty, has expectedly been while review is ongoing. (And no I don't believe in force pushing, or squashing during merges.) You can probably tell how I'm not familiar with the Github flow, and neither am fluent with most Python idioms or tooling. But am trying as hard as I can to cooperate, and provide something that is acceptable to you, and usable in the project. Hope that v3 matches your expectation. |
|
I've merged this manually. Thanks for your contribution. |
This is a cleaned up submission of the PR 673 essence. Implements transparent support for platform dependent single key press without a prompt, with Python core and without external dependencies. Supports all major desktop systems where you expect to run the debugger, falls back to the most portable backwards compatible behaviour on platforms that are known to not work with the single key approach. Does not involve config items, needs no documentation update. The implementation is considered both readable and maintainable, supporting more platforms is straight forward as they get identified. Thank you for the feedback which helped very much in getting to that state.