Skip to content

feat(controls): add copy on selection setting - #116

Merged
howdeploy merged 4 commits into
howdeploy:mainfrom
Yt4aZaveta:codex/copy-on-select
Oct 1, 2026
Merged

howdeploy merged 4 commits into
howdeploy:mainfrom
Yt4aZaveta:codex/copy-on-select

Conversation

@Yt4aZaveta

Copy link
Copy Markdown
Contributor

Summary

  • Add a persisted, off-by-default Copy on selection control setting.
  • Copy completed non-empty mouse selections for every terminal card, including fullscreen cards, while leaving search and cleared selections untouched.
  • Support Shift selection in mouse-reporting TUIs and preserve macOS canvas Option navigation through the terminal mouse adapter.
  • Document and localize the behavior in English, Russian, and Simplified Chinese.

Validation

  • Manually verified in Codex, Hermes, and a regular terminal.
  • npm run build passed.

@Yt4aZaveta Yt4aZaveta changed the title feat(terminal): add copy on selection setting feat(controls): add copy on selection setting Oct 1, 2026

Copy link
Copy Markdown
Owner

Reviewed head 03abecd700155682fe2e2b71fc4f59658fb87e28 against current main 9f5aec879a0fd557b019b2c07d63e5790410e931.

The off-by-default setting, persistence, card/fullscreen propagation, and Shift selection support are useful. There is one behavior issue to fix before merging.

P2: An ordinary TUI click can overwrite the clipboard with an old selection

In src/renderer/src/features/terminal/terminalCopyOnSelect.ts:19-33, every primary-button mousedown arms the helper, and mouseup copies whatever getSelection() currently returns. This does not establish that xterm accepted a selection gesture or that the mouse gesture produced a selection.

Concrete sequence:

  1. Enable copy on selection and use Shift to select text in a mouse-reporting TUI.
  2. Change the clipboard elsewhere.
  3. Click normally inside that TUI, without Shift.

xterm's selection service is disabled while mouse reporting is active. Its handleMouseDown returns for an unmodified click without clearing the retained selection. The application receives the mouse event, but this helper still copies the earlier text on mouseup, overwriting the clipboard. A retained search selection can trigger the same problem. This contradicts the intended behavior of copying completed mouse selections while leaving search selections untouched.

Please tie copying to a selection gesture actually accepted by xterm, rather than any click followed by a non-empty selection. Comparing only selected strings is insufficient: a new legitimate selection of the same text should still copy. Add a behavioral regression check for an unchanged retained selection plus a normal TUI click, alongside a genuine Shift-selection case. Preserve selection completion when releasing outside the screen and cancellation/disposal behavior.

Integration with current main

An actual Git merge rehearsal finds a conflict in terminalMouseCoordinates.ts. Please preserve the hover/drag-scoped document listeners introduced on main. Combine the new forceSelection path with main's endDrag() cleanup in both remapped and non-remapped mouseup paths; restoring the older unconditional listener setup would undo the optimization.

The existing CI is green for this head, but it predates the recently merged changes. Please resolve against current main and rerun CI on that resulting head.

Review method: source tracing, including the installed xterm 6.0.0 selection implementation, and Git merge checks. I did not run local tests or reproduce the scenario in the UI.

@Yt4aZaveta

Copy link
Copy Markdown
Contributor Author

@howdeploy The PR head is now merged with the current main, and the reported behavior is fixed in 7a640b4.

What changed:

  • Resolved the terminalMouseCoordinates.ts integration conflict while preserving main's hover/drag-scoped document listeners.
  • Combined the forceSelection path with endDrag() cleanup in both remapped and non-remapped mouseup paths.
  • Changed copy-on-select so mouseup copies only after xterm reports an actual selection change during the active gesture. A normal click with a retained selection no longer overwrites the clipboard, while a new selection containing the same text is still copied.
  • Wired TerminalCard to xterm's selection-change event and dispose the subscription together with the helper timers/listeners.
  • Added regression coverage for the retained-selection/ordinary-click case and the same-text genuine selection case.

Validation on the resulting head:

  • npm run build — passed
  • npm run typecheck — passed
  • node tests/terminal-copy-on-select.test.mjs — 2/2 passed
  • git diff --check — passed

Fresh GitHub CI is running for this head.

Copy link
Copy Markdown
Owner

Follow-up review of 7a640b4dac83cfe01b66e61178a3032b611d1f67:

The reported retained-selection clipboard overwrite is addressed: the helper now requires an xterm selection-change event during the gesture/completion interval, and TerminalCard disposes that subscription. The mouse-coordinate integration preserves main's scoped listeners and endDrag cleanup. Both new regression checks passed in the Linux and Windows CI logs. The previous merge conflict is resolved.

The remaining CI failures are separate:

  • Linux verify fails at tests/agent-runtime-gateway.test.mjs:430: the resumed OpenCode session never emits the expected working/session.status:busy transition. I confirmed the same failure on current main's run 36877121830. PR fix(opencode): keep resumed session bind out of turn tracking #118 addresses this bind/turn-tracking issue.
  • Windows fails during cleanup in tests/plugin-hook-runner.test.mjs:89 with EBUSY removing the temporary plugin directory. This log shows a cleanup failure, not a copy-on-select assertion failure. Please investigate a child-process/file-handle teardown race if it repeats; do not weaken the behavioral assertion.

Please sync with main once #118 lands and obtain a green run on the resulting head. The original clipboard finding is closed; the current red checks remain the merge blocker.

Source/CI-log review only; no local tests or UI checks were run.

@howdeploy
howdeploy merged commit 5bbf295 into howdeploy:main Oct 1, 2026
3 checks passed
@Yt4aZaveta
Yt4aZaveta deleted the codex/copy-on-select branch October 3, 2026 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants