gh-152907: Restore cooked output flags around the input hook in the new REPL#153389
gh-152907: Restore cooked output flags around the input hook in the new REPL#153389harjothkhara wants to merge 4 commits into
Conversation
… the new REPL pyrepl clears OPOST for its own cursor rendering but calls PyOS_InputHook from inside the raw-mode read loop, so output written by an input hook (GUI toolkit event loops, and any warning/traceback/print they emit) is emitted with bare '\n' and no '\r'. Restore the terminal's saved output flags around the hook call and re-enter raw mode afterwards; only oflag is toggled so ECHO/ICANON stay off at the prompt.
harjothkhara
requested review from
ambv,
lysnikolaou and
pablogsal
as code owners
The Emscripten buildbot has the pty module but no pty devices, so
pty.openpty() raises OSError("out of pty devices"). Guard the test
class the same way Lib/test/test_pty.py does.
| cooked.oflag = self.__svtermstate.oflag | ||
| self.__input_fd_set(cooked) | ||
| try: | ||
| posix._inputhook() |
There was a problem hiding this comment.
Can we return posix._inputhook() here? This keeps the Unix wrapper consistent with the console API and the Windows path.
| self.assertFalse(_termios.tcgetattr(slave_fd)[1] & _termios.OPOST) | ||
| finally: | ||
| console.restore() | ||
| os.close(slave_fd) |
There was a problem hiding this comment.
Nit, feel free to ignore: relying on a background reader plus time.sleep(0.2) to observe the output tends to be a source of buildbot flakiness. Since the TCSADRAIN in __run_input_hook already guarantees the bytes reach the master before the hook returns, can we drain master_fd directly after hook() instead of sleeping?
There was a problem hiding this comment.
Tried that first but it hangs on macOS — TCSADRAIN there doesn't return until the master side actually reads the output, so the mode switch blocks forever without a reader. Kept the reader thread but dropped the sleep: once the hook returns the output is already drained, so joining the reader is enough.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| return self.__run_input_hook | ||
|
|
||
| def __run_input_hook(self): | ||
| # Input-hook callbacks (GUI toolkit event loops, and any warning, |
There was a problem hiding this comment.
This comment is too verbose, pls, simplify it a bit
| @unittest.skipIf(is_android or is_apple_mobile or is_wasm32, | ||
| "pty is not available on this platform") | ||
| class TestUnixConsoleInputHook(TestCase): | ||
| # gh-152907: pyrepl runs with OPOST disabled so it can drive the cursor |
There was a problem hiding this comment.
Thhese AI comments are too verbose pls keep only the relevant parts
There was a problem hiding this comment.
Done, trimmed them down.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 participants
The problem: While the new REPL waits for input, it puts the terminal in raw mode, which turns off the usual
\n→\r\ntranslation. It also runs input hooks during that wait — these are what GUI toolkits (Tkinter, Qt, …) use to keep their event loops alive at the prompt. So when a hook prints something, each line starts where the previous one ended instead of at the left margin, and the text "staircases" down the screen. The old readline REPL didn't do this, andPYTHON_BASIC_REPL=1still works fine.The fix: The REPL needs raw mode for drawing its own output, but the input hook runs at a moment when it isn't drawing. So this turns the newline translation back on just around the hook call, then returns to raw mode. Only the output flags are touched, so keystrokes are still not echoed at the prompt.
Checked with a pty that captures the exact bytes a hook writes:
There's a new test that fails without the fix, and
./python -m test test_pyreplpasses.I used AI assistance for this and have reviewed the change.
Refs #152907