mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 22:19:42 +02:00
fix(shortcuts): stop Alt/Option nav keys leaking ESC sequences into the terminal
The PR migrated the app.js tab-nav handler to physical e.code but left xterm's pass-through gate (terminal-ui.js) matching ev.key digits. Consequences: - Alt+[ / Alt+] (the new bindings) were never in the gate, so xterm sent ESC[ / ESC] to the PTY on every platform AS WELL AS switching the session. - Alt+digit on a remapped macOS Option layout (Option+1 -> "¡") didn't match the ev.key '0'-'9' gate either, so xterm injected ESC<char> — on exactly the layouts this PR exists to fix. Update the xterm gate to mirror app.js exactly: suppress when `ev.altKey && !ctrl && !shift && /^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code)`. Returning false there tells xterm not to write to the PTY, so the shortcut switches the tab with no stray escape sequence. Also: relabel the docs Alt/Option (the mechanism is layout/OS-independent, so the shortcut works for Linux/Windows Alt users too — "Option" alone was Mac-only wording), and add a keyboard-shortcuts test asserting terminal-ui.js gates on the same physical codes so this desync can't regress (a grep the original test missed). Verified: keyboard-shortcuts test 4/4, check:frontend-syntax, check:public-assets, format:check all clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -483,8 +483,8 @@ Single-digit selection (1-9), color-coded status, token counts, auto-refresh. De
|
||||
|----------|--------|
|
||||
| `Ctrl/Cmd+W` | Kill active session |
|
||||
| `Ctrl/Cmd+Tab` | Next session |
|
||||
| `Option+[` / `Option+]` | Previous / next session |
|
||||
| `Option+1`-`Option+9` | Switch to tab N (physical keys, so macOS Option layouts work) |
|
||||
| `Alt/Option+[` / `Alt/Option+]` | Previous / next session |
|
||||
| `Alt/Option+1`-`Alt/Option+9` | Switch to tab N (physical keys, so macOS Option layouts work) |
|
||||
| `Ctrl+Shift+{` / `Ctrl+Shift+}` | Move active tab left / right |
|
||||
| `Ctrl/Cmd+L` | Clear terminal |
|
||||
| `Ctrl+Shift+R` | Restore terminal size |
|
||||
|
||||
@@ -505,8 +505,8 @@
|
||||
<div class="shortcuts-grid">
|
||||
<div><kbd>Ctrl</kbd>+<kbd>W</kbd></div><div>Close Session</div>
|
||||
<div><kbd>Ctrl</kbd>+<kbd>Tab</kbd></div><div>Next Session</div>
|
||||
<div><kbd>Option</kbd>+<kbd>[</kbd> / <kbd>Option</kbd>+<kbd>]</kbd></div><div>Previous / Next Session</div>
|
||||
<div><kbd>Option</kbd>+<kbd>1-9</kbd></div><div>Switch to Tab N</div>
|
||||
<div><kbd>Alt/Option</kbd>+<kbd>[</kbd> / <kbd>Alt/Option</kbd>+<kbd>]</kbd></div><div>Previous / Next Session</div>
|
||||
<div><kbd>Alt/Option</kbd>+<kbd>1-9</kbd></div><div>Switch to Tab N</div>
|
||||
<div><kbd>Ctrl</kbd>+<kbd>L</kbd></div><div>Clear Terminal</div>
|
||||
<div><kbd>Ctrl</kbd>+<kbd>+</kbd></div><div>Increase Font</div>
|
||||
<div><kbd>Ctrl</kbd>+<kbd>-</kbd></div><div>Decrease Font</div>
|
||||
|
||||
@@ -114,8 +114,14 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.terminal.attachCustomKeyEventHandler((ev) => {
|
||||
if (ev.isComposing || ev.keyCode === 229) return false;
|
||||
|
||||
// Let Alt+digit pass through to browser (tab switching)
|
||||
if (ev.altKey && ev.key >= '0' && ev.key <= '9') return false;
|
||||
// Let the app's Alt/Option session-nav shortcuts reach the document keydown handler
|
||||
// (app.js switches tabs by PHYSICAL e.code) instead of xterm injecting ESC<char> into
|
||||
// the PTY. Mirror app.js's gate exactly — same physical codes + modifier guard — so
|
||||
// macOS Option layouts (Option+1 -> "¡", Option+[ -> "“") are suppressed here too and
|
||||
// don't leak an escape sequence into the focused terminal on every tab switch.
|
||||
if (ev.altKey && !ev.ctrlKey && !ev.shiftKey && /^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code || '')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Ctrl+V / Cmd+V: intercept before xterm sends ^V to PTY.
|
||||
// Route through our paste trap which handles both images and text.
|
||||
|
||||
@@ -2,6 +2,7 @@ import { readFileSync } from 'node:fs';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
const appSource = readFileSync('src/web/public/app.js', 'utf8');
|
||||
const terminalUiSource = readFileSync('src/web/public/terminal-ui.js', 'utf8');
|
||||
const helpHtml = readFileSync('src/web/public/index.html', 'utf8');
|
||||
const readme = readFileSync('README.md', 'utf8');
|
||||
|
||||
@@ -19,11 +20,18 @@ describe('keyboard shortcuts', () => {
|
||||
expect(appSource).toContain('this.nextSession()');
|
||||
});
|
||||
|
||||
it('documents mac-friendly Option shortcuts in help and README', () => {
|
||||
expect(helpHtml).toContain('<kbd>Option</kbd>+<kbd>[</kbd>');
|
||||
expect(helpHtml).toContain('<kbd>Option</kbd>+<kbd>]</kbd>');
|
||||
expect(helpHtml).toContain('<kbd>Option</kbd>+<kbd>1-9</kbd>');
|
||||
expect(readme).toContain('`Option+[` / `Option+]`');
|
||||
expect(readme).toContain('`Option+1`-`Option+9`');
|
||||
it('suppresses xterm PTY injection for the same physical Alt nav codes (no ESC leak)', () => {
|
||||
// terminal-ui.js must gate its xterm pass-through on the SAME physical e.code set the
|
||||
// app.js handler consumes; otherwise Alt+[ / Alt+] (and Option+digit on remapped macOS
|
||||
// layouts) switch tabs AND inject ESC<char> into the focused terminal. Keep in sync.
|
||||
expect(terminalUiSource).toContain('/^(Digit[1-9]|BracketLeft|BracketRight)$/.test(ev.code');
|
||||
});
|
||||
|
||||
it('documents the Alt/Option shortcuts in help and README', () => {
|
||||
expect(helpHtml).toContain('<kbd>Alt/Option</kbd>+<kbd>[</kbd>');
|
||||
expect(helpHtml).toContain('<kbd>Alt/Option</kbd>+<kbd>]</kbd>');
|
||||
expect(helpHtml).toContain('<kbd>Alt/Option</kbd>+<kbd>1-9</kbd>');
|
||||
expect(readme).toContain('`Alt/Option+[` / `Alt/Option+]`');
|
||||
expect(readme).toContain('`Alt/Option+1`-`Alt/Option+9`');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user