fix(cli): use get_wch() for CJK/Unicode input in curses session browser - #40458
fix(cli): use get_wch() for CJK/Unicode input in curses session browser#40458liuhao1024 wants to merge 3 commits into
Conversation
Replace stdscr.getch() with stdscr.get_wch() in _session_browse_picker and _run_curses_menu to support CJK (Korean, Chinese, Japanese) and emoji input. getch() only returns byte values (0-255), silently dropping all Unicode characters. get_wch() returns proper Unicode strings. Key comparisons updated to handle both integer codes (from getch() fallback) and string characters (from get_wch()): Enter, Esc, Backspace, Ctrl+U, and quit key. Fallback to getch() via AttributeError for curses builds without wide-char support. Fixes NousResearch#40446
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Replaces getch() with get_wch() in cli.py for proper CJK/Unicode character capture in the curses session browser. This is the correct fix for wide-character input on platforms that support ncurses wide-character functions.
Looks Good
- Addresses a real Unicode pain point in the TUI
- Change is contained to the input capture path
- No regression for ASCII input —
get_wch()handles both
Reviewed by Hermes Agent
The PR replaced stdscr.getch() with stdscr.get_wch() for CJK/Unicode support, but the test mock only intercepted getch(). The default MagicMock.get_wch() returned a MagicMock object instead of raising AttributeError, so the curses input loop never matched any key and hung until the 30s pytest timeout.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the separate curses session-browser path; current main still has the reported ASCII-only input gate at hermes_cli/main.py:1083-1115.
Problems
- In the shared searchable-menu path, this changes input to
get_wch()but leaves the search opener askey == ord("/")(hermes_cli/curses_ui.py:501in the PR context).get_wch()returns printable/as a string, so/is ignored and search cannot be activated. The same integer-only assumptions remain in_decode_menu_key. tests/hermes_cli/test_curses_cjk_input.pytests_handle_active_search_keydirectly, but does not cover the_run_curses_menuloop receiving string keys fromget_wch().
Suggested changes
- Handle both
"/"andord("/"), and normalize or extend_decode_menu_keyfor string character keys. - Add an end-to-end mocked shared-menu search test using string
get_wch()values.
Automated hermes-sweeper review.
| @@ -475,7 +479,10 @@ def _draw(stdscr): | |||
| stdscr.refresh() | |||
|
|
|||
| if use_search: | |||
There was a problem hiding this comment.
get_wch() returns printable keys as strings. The following unchanged branch checks key == ord("/"), so / no longer opens search in this loop. Please normalize string/int keys here (and in _decode_menu_key) and add a loop-level regression test.
What does this PR do?
Fixes CJK (Korean, Chinese, Japanese) and emoji input in the curses-based session browser (
hermes sessions browse) and the shared curses menu. Previously, only printable ASCII characters (key codes 32–126) were accepted — all Unicode input was silently ignored.Related Issue
Fixes #40446
Type of Change
Changes Made
hermes_cli/main.py: In_session_browse_picker, replacedstdscr.getch()withstdscr.get_wch()(withAttributeErrorfallback for platforms lacking wide-char support). Updated key comparisons to accept both integer codes (fromgetch()) and string characters (fromget_wch()): Enter ("\n","\r"), Esc ("\x1b"), Backspace ("\x7f"), quit ("q"). Addedisinstance(key, str) and key.isprintable()branch for Unicode character input.hermes_cli/curses_ui.py: In_run_curses_menu, replacedstdscr.getch()withget_wch()+ fallback. In_handle_active_search_key, updated type annotation toint | str, and updated all key comparisons (Esc, Backspace, Ctrl+U, Enter, printable chars) to handle string keys fromget_wch().tests/hermes_cli/test_curses_cjk_input.py: 14 regression tests — 8 unit tests for_handle_active_search_key(Korean, Chinese, Japanese, emoji, ASCII fallback, non-printable rejection, Esc, special keys) and 6 integration tests for_session_browse_picker(Korean/Chinese/Japanese/emoji filtering, multi-char CJK search,getch()fallback).How to Test
pytest tests/hermes_cli/test_curses_cjk_input.py -xvs— all 14 tests should passhermes sessions browsein a terminal with Korean/Chinese/Japanese input method enabledChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/Aget_wch()is wrapped in try/except AttributeError for graceful fallback on platforms without wide-char curses supportCode Intelligence
hermes_cli/main.py:_session_browse_picker(session browser input loop),hermes_cli/curses_ui.py:_handle_active_search_key(shared menu search handler),hermes_cli/curses_ui.py:_run_curses_menu(shared menu key reading)getch()preserves existing behavior on all platformsgetch()returns integer key codes (0–255);get_wch()returns strings for Unicode characters and integers for special keys. All key comparisons must handle both types.