Conversation
There was a problem hiding this comment.
Pull request overview
Adds /copy command support across the classic Python CLI and the Ink-based TUI, including interactive selection of fenced code blocks and improved clipboard handling.
Changes:
- Classic CLI: implement code-block extraction, interactive picker, native clipboard copy with OSC52 fallback, and “write to file” flow.
- TUI: add a copy-picker overlay state/UI + keyboard handling, and enhance
/copyto open the picker when code blocks are detected. - Tests: expand
/copycoverage for clipboard fallback behavior, code-block parsing, picker flows, and non-TTY fallback.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
cli.py |
Adds code-block extraction + picker and updates clipboard copy behavior (native-first, OSC52 fallback). |
hermes_cli/curses_ui.py |
Introduces curses_copy_picker and a numbered fallback UI. |
hermes_cli/commands.py |
Updates /copy help text to mention the interactive picker. |
tests/cli/test_cli_copy_command.py |
Adds/updates tests for new /copy behavior, picker, and clipboard paths. |
ui-tui/src/lib/text.ts |
Adds extractCodeBlocks() helper for the TUI /copy picker. |
ui-tui/src/app/interfaces.ts |
Adds CopyPickerState / CopyPickerItem types and overlay state field. |
ui-tui/src/app/overlayStore.ts |
Initializes copyPicker and includes it in the blocked-overlay computation. |
ui-tui/src/app/slash/commands/core.ts |
Updates /copy to open the overlay picker when code blocks are present. |
ui-tui/src/app/useInputHandlers.ts |
Adds navigation/selection/cancel handling for the copy picker overlay. |
ui-tui/src/components/appOverlays.tsx |
Renders the copy picker overlay UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const CODE_BLOCK_RE = /```(\w*)\n([\s\S]*?)```/g | ||
|
|
||
| export interface CodeBlock { | ||
| code: string | ||
| lang: string | ||
| } | ||
|
|
||
| export const extractCodeBlocks = (text: string): CodeBlock[] => { | ||
| const blocks: CodeBlock[] = [] | ||
|
|
||
| for (const m of text.matchAll(CODE_BLOCK_RE)) { | ||
| blocks.push({ code: m[2]!.replace(/\n$/, ''), lang: m[1] || 'text' }) |
There was a problem hiding this comment.
extractCodeBlocks is unlikely to detect many valid Markdown fenced code blocks because CODE_BLOCK_RE only allows \w* languages and only fences. This will miss common info strings like `shell-session`, `c++`, `tsconfig.json`, or fences with additional attributes/spaces, and it won’t match `~~~` fences (which the TUI markdown renderer and `estimateRows` already support). Consider reusing the existing fence parsing approach (variable-length/~~~ + trimming the full info string) so the /copy picker triggers consistently.
| r"```(\w*)\n(.*?)```", re.DOTALL | ||
| ) | ||
|
|
||
|
|
||
| def _extract_code_blocks(text: str) -> list[tuple[str, str]]: | ||
| """Return ``[(lang, code), ...]`` for every fenced code block in *text*.""" | ||
| return [ | ||
| (m.group(1) or "text", m.group(2).rstrip("\n")) |
There was a problem hiding this comment.
The fenced-code regex _CODE_BLOCK_RE only matches language tags made of \w* and requires the info string to be immediately followed by \n. This will fail to extract many real-world fenced blocks like shell-session, c++, or ```python linenos (and any fence using ~~~). Consider loosening the info-string capture (e.g., capture and trim everything up to the newline) and/or sharing the same fence parsing rules used elsewhere in the project so `/copy` reliably detects code blocks.
| r"```(\w*)\n(.*?)```", re.DOTALL | |
| ) | |
| def _extract_code_blocks(text: str) -> list[tuple[str, str]]: | |
| """Return ``[(lang, code), ...]`` for every fenced code block in *text*.""" | |
| return [ | |
| (m.group(1) or "text", m.group(2).rstrip("\n")) | |
| r"^(?P<fence>`{3,}|~{3,})(?P<info>[^\n]*)\n(?P<code>.*?)(?:^(?P=fence)[ \t]*$)", | |
| re.DOTALL | re.MULTILINE, | |
| ) | |
| def _extract_code_blocks(text: str) -> list[tuple[str, str]]: | |
| """Return ``[(lang, code), ...]`` for every fenced code block in *text*.""" | |
| return [ | |
| ( | |
| (m.group("info").strip().split(None, 1)[0] if m.group("info").strip() else "text"), | |
| m.group("code").rstrip("\n"), | |
| ) |
| ))} | ||
|
|
||
| <Box marginTop={1}> | ||
| <Text color={ui.theme.color.dim}>↑↓ navigate · Enter copy · Esc cancel</Text> |
There was a problem hiding this comment.
The copy picker overlay advertises only "↑↓ navigate · Enter copy · Esc cancel", but useInputHandlers also implements a w key path (with different behavior/message). Either the hint should mention w, or the w handler should be removed/changed so the UI and controls stay in sync.
| <Text color={ui.theme.color.dim}>↑↓ navigate · Enter copy · Esc cancel</Text> | |
| <Text color={ui.theme.color.dim}>↑↓ navigate · Enter copy · w copy to file · Esc cancel</Text> |
| mock_copy.assert_not_called() | ||
| assert any("Invalid response number" in str(call) for call in mock_print.call_args_list) |
There was a problem hiding this comment.
The comprehension for call in mock_print.call_args_list shadows the call symbol imported from unittest.mock (and the import is otherwise unused). This is confusing and may trip linting; consider renaming the loop variable and dropping the unused call import.
|
Closing as superseded by #20159. Triage notes (high confidence): Thanks for the contribution — the underlying problem this PR addresses has been resolved by the linked PR on current main. If you believe this was closed in error, please comment and we'll reopen. (Bulk-closed during a CLI PR triage sweep.) |
What does this PR do?
Add /copy support
Related Issue
Fixes #11835
Type of Change
Changes Made
Here are all the changes made for this feature:
Python — Classic CLI
cli.py_CODE_BLOCK_REregex and_extract_code_blocks()helper (line ~119) to parse fenced code blocks from markdown text_write_osc52_clipboard()to try native clipboard (pbcopy/xclip/xsel) first, falling back to OSC 52 only over SSH_copy_to_system_clipboard()method — runspbcopyon macOS,wl-copy/xclip/xselon Linux via subprocess_handle_copy_command()— detects code blocks and delegates to the interactive picker when present_copy_with_picker()— builds picker labels from code blocks and callscurses_copy_picker_copy_text_to_clipboard()— extracted clipboard copy + confirmation message_write_copy_to_file()— prompts for file path (with smart extension default) and writes selected contenthermes_cli/curses_ui.pycurses_copy_picker()— curses-based single-select with Enter (copy) /w(write to file) / Esc (cancel)_copy_picker_numbered_fallback()— text-based fallback for non-curses terminalshermes_cli/commands.pyCommandDefdescription for/copyto mention the interactive pickerTypeScript — TUI (Ink)
ui-tui/src/app/interfaces.tsCopyPickerStateandCopyPickerIteminterfacescopyPicker: CopyPickerState | nulltoOverlayStateui-tui/src/app/overlayStore.tscopyPicker: nullto default overlay statecopyPickerin the$isBlockedcomputed storeui-tui/src/app/useInputHandlers.tsui-tui/src/components/appOverlays.tsxFloatingOverlayswith cursor highlighthasAnyguard to includecopyPickerui-tui/src/app/slash/commands/core.ts/copyrun handler to extract code blocks and open the picker overlay when blocks are presentui-tui/src/lib/text.tsCodeBlockinterface andextractCodeBlocks()functionTests
tests/cli/test_cli_copy_command.py_copy_to_system_clipboardmock instead of_write_osc52_clipboardtest_copy_falls_back_to_osc52_when_native_unavailabletest_extract_code_blocks_finds_fenced_blocks,test_extract_code_blocks_no_lang_defaults_to_text,test_extract_code_blocks_returns_empty_for_no_blockstest_copy_opens_picker_when_code_blocks_present,test_copy_no_picker_when_no_code_blockstest_copy_with_picker_copies_full_response,test_copy_with_picker_copies_specific_block,test_copy_with_picker_cancel,test_copy_with_picker_write_to_filetest_curses_copy_picker_fallback_non_ttytest_system_clipboard_uses_pbcopy_on_macos,test_system_clipboard_skipped_over_sshHow to Test
Checklist
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/AFor New Skills
hermes --toolsets skills -q "Use the X skill to do Y"Screenshots / Logs