mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 16:39:42 +02:00
fix(review): gate wheel forwarding by CLI mode/version, stop link click double-fire (PR #144)
- _shouldForwardWheelToApp: claude sessions forward wheel to the TUI only when the banner-parsed cliVersion is known AND >= 2.1.187 (older/unknown Claude Code captures wheel as select-menu navigation → keep local scrollLines); new dependency-free _cliVersionAtLeast semver-ish compare - gemini excluded from wheel forwarding entirely (TUI wheel behavior unverified); codex keeps forwarding (verified); taps/clicks still forwarded for all strip modes - link double-fire: registerFilePathLinkProvider links now track hover state via ILink hover/leave callbacks (_linkHovered) and _handleDesktopTerminalClick bails while a link is hovered, so a link click no longer also sends a synthetic SGR press/release to the TUI - help modal: document Shift+Wheel (scroll local history when mouse passthrough is active) - tests: version gate (2.1.186/unknown/garbage no forward, 2.1.187+ forwards), codex/gemini split, link-hover click suppression Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -560,6 +560,7 @@
|
|||||||
<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>[</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>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>L</kbd></div><div>Clear Terminal</div>
|
||||||
|
<div><kbd>Shift</kbd>+<kbd>Wheel</kbd></div><div>Scroll local history (when mouse passthrough is active)</div>
|
||||||
<div><kbd>Ctrl</kbd>+<kbd>+</kbd></div><div>Increase Font</div>
|
<div><kbd>Ctrl</kbd>+<kbd>+</kbd></div><div>Increase Font</div>
|
||||||
<div><kbd>Ctrl</kbd>+<kbd>-</kbd></div><div>Decrease Font</div>
|
<div><kbd>Ctrl</kbd>+<kbd>-</kbd></div><div>Decrease Font</div>
|
||||||
<div><kbd>Ctrl</kbd>+<kbd>?</kbd></div><div>Show Help</div>
|
<div><kbd>Ctrl</kbd>+<kbd>?</kbd></div><div>Show Help</div>
|
||||||
|
|||||||
@@ -323,12 +323,14 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
// Register link provider for clickable file paths in Bash tool output
|
// Register link provider for clickable file paths in Bash tool output
|
||||||
this.registerFilePathLinkProvider();
|
this.registerFilePathLinkProvider();
|
||||||
|
|
||||||
// Mouse wheel: forward to the TUI for strip-mode sessions, local scrollback
|
// Mouse wheel: forward to the TUI only for sessions verified to handle SGR
|
||||||
// otherwise. Claude Code (2.1.187+) scrolls its own transcript on SGR wheel
|
// wheel reports (codex, and claude 2.1.187+ — see _shouldForwardWheelToApp),
|
||||||
// reports — scrolled-away tool blocks re-render live and stay clickable —
|
// local scrollback otherwise. Claude Code 2.1.187+ scrolls its own
|
||||||
// and its select menus no longer capture wheel as option navigation
|
// transcript on SGR wheel reports — scrolled-away tool blocks re-render
|
||||||
// (verified against 2.1.202: /model menu highlight ignores wheel reports),
|
// live and stay clickable — and its select menus no longer capture wheel
|
||||||
// so the original reason to keep the wheel local is gone for claude mode.
|
// as option navigation (verified against 2.1.202: /model menu highlight
|
||||||
|
// ignores wheel reports); older versions DO capture wheel as option
|
||||||
|
// navigation, so they keep the local wheel.
|
||||||
// Shift+wheel always scrolls xterm's local scrollback (Codeman's restored
|
// Shift+wheel always scrolls xterm's local scrollback (Codeman's restored
|
||||||
// history lives there), and once the viewport left the bottom the wheel
|
// history lives there), and once the viewport left the bottom the wheel
|
||||||
// stays local until the user scrolls back down — so both scrollbacks stay
|
// stays local until the user scrolls back down — so both scrollbacks stay
|
||||||
@@ -931,6 +933,12 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
activate(_event, text) {
|
activate(_event, text) {
|
||||||
window.open(text, '_blank', 'noopener,noreferrer');
|
window.open(text, '_blank', 'noopener,noreferrer');
|
||||||
},
|
},
|
||||||
|
hover() {
|
||||||
|
self._linkHovered = true;
|
||||||
|
},
|
||||||
|
leave() {
|
||||||
|
self._linkHovered = false;
|
||||||
|
},
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -969,6 +977,12 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
activate(event, text) {
|
activate(event, text) {
|
||||||
self.openLogViewerWindow(text, self.activeSessionId);
|
self.openLogViewerWindow(text, self.activeSessionId);
|
||||||
},
|
},
|
||||||
|
hover() {
|
||||||
|
self._linkHovered = true;
|
||||||
|
},
|
||||||
|
leave() {
|
||||||
|
self._linkHovered = false;
|
||||||
|
},
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
@@ -2146,13 +2160,39 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this._sendInputAsync(this.activeSessionId, `\x1b[<0;${pos.col};${pos.row}M\x1b[<0;${pos.col};${pos.row}m`);
|
this._sendInputAsync(this.activeSessionId, `\x1b[<0;${pos.col};${pos.row}M\x1b[<0;${pos.col};${pos.row}m`);
|
||||||
},
|
},
|
||||||
|
|
||||||
// Wheel forwarding gate for the container wheel handler: strip-mode session,
|
// True when a parsed CLI version string ('2.1.187' — banner-parsed on the
|
||||||
// no Shift override, xterm's own encoder dormant, viewport at the bottom.
|
// server, delivered via session:cliInfo / SessionState.cliVersion) is known
|
||||||
|
// AND >= the minimum. Unknown or unparseable versions return false so
|
||||||
|
// callers keep the conservative behavior.
|
||||||
|
_cliVersionAtLeast(version, minimum) {
|
||||||
|
if (typeof version !== 'string') return false;
|
||||||
|
const parts = version.trim().replace(/^v/, '').split('.').map(Number);
|
||||||
|
if (parts.length !== 3 || parts.some((n) => !Number.isFinite(n))) return false;
|
||||||
|
const min = minimum.split('.').map(Number);
|
||||||
|
for (let i = 0; i < 3; i++) {
|
||||||
|
if (parts[i] !== min[i]) return parts[i] > min[i];
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
},
|
||||||
|
|
||||||
|
// Wheel forwarding gate for the container wheel handler: no Shift override,
|
||||||
|
// xterm's own encoder dormant, viewport at the bottom, and a TUI VERIFIED to
|
||||||
|
// scroll its transcript on SGR wheel reports: codex, or claude 2.1.187+
|
||||||
|
// (older Claude Code captures wheel as select-menu option navigation; an
|
||||||
|
// unknown version is treated as older). Gemini is a strip mode too but its
|
||||||
|
// wheel behavior is unverified, so it keeps the local wheel — taps/clicks
|
||||||
|
// are still forwarded for it (harmless no-ops at worst).
|
||||||
_shouldForwardWheelToApp(ev) {
|
_shouldForwardWheelToApp(ev) {
|
||||||
if (ev.shiftKey) return false;
|
if (ev.shiftKey) return false;
|
||||||
const mode = this.terminal?.modes?.mouseTrackingMode;
|
const mode = this.terminal?.modes?.mouseTrackingMode;
|
||||||
if (mode && mode !== 'none') return false;
|
if (mode && mode !== 'none') return false;
|
||||||
if (!this._sessionUsesServerMouseStrip()) return false;
|
const session = this.sessions?.get(this.activeSessionId);
|
||||||
|
const sessionMode = session?.mode || 'claude';
|
||||||
|
if (sessionMode === 'claude') {
|
||||||
|
if (!this._cliVersionAtLeast(session?.cliVersion, '2.1.187')) return false;
|
||||||
|
} else if (sessionMode !== 'codex') {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
return this._terminalViewportAtBottom();
|
return this._terminalViewportAtBottom();
|
||||||
},
|
},
|
||||||
|
|
||||||
@@ -2189,6 +2229,8 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
// a meaning elsewhere: synthetic/compat clicks after a touch tap (touchend
|
// a meaning elsewhere: synthetic/compat clicks after a touch tap (touchend
|
||||||
// reported already), modified clicks (shift keeps xterm's selection
|
// reported already), modified clicks (shift keeps xterm's selection
|
||||||
// override), double/triple clicks (word/line selection), drag-selections,
|
// override), double/triple clicks (word/line selection), drag-selections,
|
||||||
|
// clicks on hovered links (activate() already handles the click — a second
|
||||||
|
// synthetic SGR press could e.g. dismiss a claude permission dialog),
|
||||||
// clicks outside the cell grid, and sessions where xterm's own encoder is
|
// clicks outside the cell grid, and sessions where xterm's own encoder is
|
||||||
// live (it reported the click itself — a second report would double-move).
|
// live (it reported the click itself — a second report would double-move).
|
||||||
_handleDesktopTerminalClick(ev) {
|
_handleDesktopTerminalClick(ev) {
|
||||||
@@ -2199,6 +2241,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
if (mode && mode !== 'none') return;
|
if (mode && mode !== 'none') return;
|
||||||
if (!this._sessionUsesServerMouseStrip()) return;
|
if (!this._sessionUsesServerMouseStrip()) return;
|
||||||
if (this.terminal.hasSelection?.()) return;
|
if (this.terminal.hasSelection?.()) return;
|
||||||
|
if (this._linkHovered) return; // link provider hover/leave callbacks (registerFilePathLinkProvider)
|
||||||
if (performance.now() <= (this._trustedTapMouseSuppressUntil || 0)) return;
|
if (performance.now() <= (this._trustedTapMouseSuppressUntil || 0)) return;
|
||||||
if (!ev.target?.closest?.('.xterm-screen')) return;
|
if (!ev.target?.closest?.('.xterm-screen')) return;
|
||||||
this._sendSyntheticSgrTap(ev.clientX, ev.clientY);
|
this._sendSyntheticSgrTap(ev.clientX, ev.clientY);
|
||||||
|
|||||||
@@ -219,6 +219,40 @@ describe('terminal touch tap mouse guard', () => {
|
|||||||
expect(sent).toEqual([]);
|
expect(sent).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('desktop click: skips the click while a terminal link is hovered (activate() handles it)', () => {
|
||||||
|
const { app } = loadTerminalUiHarness();
|
||||||
|
const sent: string[] = [];
|
||||||
|
app.activeSessionId = 'sess-1';
|
||||||
|
app.sessions = new Map([['sess-1', { mode: 'claude' }]]);
|
||||||
|
app._sendInputAsync = (_id: string, data: string) => sent.push(data);
|
||||||
|
app.terminal = {
|
||||||
|
cols: 80,
|
||||||
|
rows: 24,
|
||||||
|
modes: { mouseTrackingMode: 'none' },
|
||||||
|
hasSelection: () => false,
|
||||||
|
element: {
|
||||||
|
querySelector: () => ({ getBoundingClientRect: () => ({ left: 0, top: 0 }) }),
|
||||||
|
},
|
||||||
|
_core: { _renderService: { dimensions: { css: { cell: { width: 8, height: 16 } } } } },
|
||||||
|
};
|
||||||
|
const click = {
|
||||||
|
isTrusted: true,
|
||||||
|
button: 0,
|
||||||
|
detail: 1,
|
||||||
|
clientX: 50,
|
||||||
|
clientY: 50,
|
||||||
|
target: { closest: (sel: string) => (sel === '.xterm-screen' ? {} : null) },
|
||||||
|
};
|
||||||
|
|
||||||
|
app._linkHovered = true; // link provider hover() fired — this click opens the link
|
||||||
|
app._handleDesktopTerminalClick(click);
|
||||||
|
expect(sent).toEqual([]);
|
||||||
|
|
||||||
|
app._linkHovered = false; // leave() fired — plain clicks report again
|
||||||
|
app._handleDesktopTerminalClick(click);
|
||||||
|
expect(sent).toEqual(['\x1b[<0;7;4M\x1b[<0;7;4m']);
|
||||||
|
});
|
||||||
|
|
||||||
it('desktop click: skips the compat click that follows a touch tap', () => {
|
it('desktop click: skips the compat click that follows a touch tap', () => {
|
||||||
const { app, setNow } = loadTerminalUiHarness();
|
const { app, setNow } = loadTerminalUiHarness();
|
||||||
const sent: string[] = [];
|
const sent: string[] = [];
|
||||||
@@ -277,10 +311,10 @@ describe('terminal touch tap mouse guard', () => {
|
|||||||
expect(sent).toEqual(['\x1b[<0;7;4M\x1b[<0;7;4m']);
|
expect(sent).toEqual(['\x1b[<0;7;4M\x1b[<0;7;4m']);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('wheel: forwards to the app only for strip-mode sessions at the buffer bottom without Shift', () => {
|
it('wheel: forwards to the app only for verified sessions at the buffer bottom without Shift', () => {
|
||||||
const { app } = loadTerminalUiHarness();
|
const { app } = loadTerminalUiHarness();
|
||||||
app.activeSessionId = 'sess-1';
|
app.activeSessionId = 'sess-1';
|
||||||
app.sessions = new Map([['sess-1', { mode: 'claude' }]]);
|
app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion: '2.1.187' }]]);
|
||||||
app.terminal = {
|
app.terminal = {
|
||||||
modes: { mouseTrackingMode: 'none' },
|
modes: { mouseTrackingMode: 'none' },
|
||||||
buffer: { active: { viewportY: 50, baseY: 50 } },
|
buffer: { active: { viewportY: 50, baseY: 50 } },
|
||||||
@@ -301,6 +335,41 @@ describe('terminal touch tap mouse guard', () => {
|
|||||||
expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false);
|
expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('wheel: gates claude forwarding on CLI version 2.1.187+ (unknown or older stays local)', () => {
|
||||||
|
const { app } = loadTerminalUiHarness();
|
||||||
|
app.activeSessionId = 'sess-1';
|
||||||
|
app.terminal = {
|
||||||
|
modes: { mouseTrackingMode: 'none' },
|
||||||
|
buffer: { active: { viewportY: 50, baseY: 50 } },
|
||||||
|
};
|
||||||
|
const withVersion = (cliVersion?: string) => {
|
||||||
|
app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion }]]);
|
||||||
|
return app._shouldForwardWheelToApp({ shiftKey: false });
|
||||||
|
};
|
||||||
|
|
||||||
|
expect(withVersion(undefined)).toBe(false); // banner not parsed yet → assume older
|
||||||
|
expect(withVersion('2.1.186')).toBe(false); // last version whose menus capture wheel
|
||||||
|
expect(withVersion('2.1.187')).toBe(true); // first version verified safe
|
||||||
|
expect(withVersion('2.2.0')).toBe(true);
|
||||||
|
expect(withVersion('3.0.0')).toBe(true);
|
||||||
|
expect(withVersion('garbage')).toBe(false); // unparseable → assume older
|
||||||
|
});
|
||||||
|
|
||||||
|
it('wheel: codex forwards without a version; gemini never forwards', () => {
|
||||||
|
const { app } = loadTerminalUiHarness();
|
||||||
|
app.activeSessionId = 'sess-1';
|
||||||
|
app.terminal = {
|
||||||
|
modes: { mouseTrackingMode: 'none' },
|
||||||
|
buffer: { active: { viewportY: 50, baseY: 50 } },
|
||||||
|
};
|
||||||
|
|
||||||
|
app.sessions = new Map([['sess-1', { mode: 'codex' }]]); // verified TUI, no version gate
|
||||||
|
expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(true);
|
||||||
|
|
||||||
|
app.sessions = new Map([['sess-1', { mode: 'gemini', cliVersion: '9.9.9' }]]); // unverified TUI
|
||||||
|
expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
it('wheel: encodes SGR 64/65 ticks, caps per event, and coalesces into one flush', () => {
|
it('wheel: encodes SGR 64/65 ticks, caps per event, and coalesces into one flush', () => {
|
||||||
const { app } = loadTerminalUiHarness();
|
const { app } = loadTerminalUiHarness();
|
||||||
const sent: Array<{ id: string; data: string }> = [];
|
const sent: Array<{ id: string; data: string }> = [];
|
||||||
|
|||||||
Reference in New Issue
Block a user