fix(cli): preserve Ctrl-J newline on macOS - #27688
Conversation
|
Thanks for the focused fix. I verified the underlying bug still exists on current main: Problems
Suggested changes
This is an automated hermes-sweeper review. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused classic-CLI regression coverage. The macOS premise still holds on current main: local macOS falls through _preserve_ctrl_enter_newline() at cli.py:3149-3171, so _bind_prompt_submit_keys() binds c-j to submit at cli.py:3189-3190, while the newline handler is conditional at cli.py:13583-13597.
Problems
- Current main added Ghostty preservation at
cli.py:3155-3159incf8862cfa; this PR branch predates those checks. Preserve them while addingdarwin, or Ghostty/tmux newline behavior would regress. - Adding macOS also requires updating the Windows-only descriptions at
cli.py:3181-3186andcli.py:13586-13595. - The PR-head predicate docstring at
cli.py:2141-2142refers to “Danny's MacBook”; use neutral platform/terminal wording instead.
Suggested changes
- Salvage the darwin branch into the current predicate without dropping the Ghostty branches from
cf8862cfa. - Update both current inline keybinding docstrings to name native macOS.
- Generalize the user-specific wording.
This is an automated hermes-sweeper review.
| See issue #22379. | ||
| """ | ||
| if sys.platform == "win32": | ||
| if sys.platform in {"darwin", "win32"}: |
There was a problem hiding this comment.
When salvaging this onto current main, retain the later Ghostty checks added by cf8862cfa (current cli.py:3155-3159). This branch predates them, and replacing the predicate wholesale would make Ghostty/tmux c-j submit again.
| some thin PTYs without SSH) still need c-j bound to submit, so we keep | ||
| that binding for those. | ||
| Native macOS, native Windows, WSL, SSH sessions, and Windows Terminal all | ||
| need c-j kept free for the newline handler. On Danny's MacBook, Ctrl+J is |
There was a problem hiding this comment.
Please replace the user-specific “Danny's MacBook” explanation with neutral wording about native macOS terminals and bare LF/c-j delivery.
Summary
Test plan
.venv/bin/python -m pytest -o addopts="-m 'not integration'" -q tests/cli/test_ctrl_enter_newline.py tests/cli/test_cli_init.py tests/hermes_cli/test_tips.pymacos_ctrl_j_preserve_and_keybinding_assertion=PASSgit diff --check