Skip to content

Commit 74083e7

Browse files
sarg3ntclaude
andcommitted
address Copilot review findings on PR #143
- Global Ctrl-Shift-` shortcut now reads the correct cookie name (gearbox_active_box, not box_id) and wires eagerly on DOMContentLoaded so it works before the user has opened the console once. - Add .console-search-bar.hidden and .console-paste-modal.hidden to the .hidden specificity-override block; Tailwind's .hidden was losing the cascade tie to our component display rules. - Paste threshold now counts actual newline characters (>= 20) rather than split('\n').length, removing the off-by-one and the trailing-newline edge case. Visible "N lines" label reads newlines + 1 to match how users count. - Confirmed paste routes through term.paste(text) so xterm applies its normal paste pipeline including bracketed-paste wrapping (ESC[200~ ... ESC[201~) when the remote shell has DECSET 2004 enabled. Direct sendText was bypassing that. - Drop dead CSS rules targeting .xterm-decoration.console-search-*. The xterm-addon-search uses its own class names (xterm-find-result-decoration / xterm-find-active-result-decoration) and we already pass decorations.{activeMatchBackground,matchBackground} options in JS, so the CSS was unreachable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 656da21 commit 74083e7

2 files changed

Lines changed: 51 additions & 24 deletions

File tree

gearbox/static/css/components/console.css

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ body.console-dock-open {
6565
.console-tool-btn.hidden,
6666
.console-tab-dot.hidden,
6767
.console-tab-new.hidden,
68-
.console-tab-bar.hidden {
68+
.console-tab-bar.hidden,
69+
.console-search-bar.hidden,
70+
.console-paste-modal.hidden {
6971
display: none;
7072
}
7173

@@ -292,9 +294,6 @@ body.console-dock-open {
292294
justify-content: center;
293295
padding: 24px;
294296
}
295-
.console-paste-modal.hidden {
296-
display: none;
297-
}
298297
.console-paste-modal-panel {
299298
background: rgb(15, 23, 42);
300299
border: 1px solid rgb(51, 65, 85);
@@ -358,14 +357,6 @@ body.console-dock-open {
358357
background: rgb(110, 231, 183);
359358
}
360359

361-
/* xterm-addon-search highlight tweaks for our dark theme */
362-
.xterm .xterm-decoration.console-search-active {
363-
background: rgba(250, 204, 21, 0.6);
364-
}
365-
.xterm .xterm-decoration.console-search-other {
366-
background: rgba(250, 204, 21, 0.25);
367-
}
368-
369360
/* ---------- xterm padding ---------- */
370361

371362
/* xterm.js paints right up to its container's edges by default. The

gearbox/static/js/console/console.js

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -561,12 +561,15 @@
561561

562562
/* ---------- paste confirm ---------- */
563563

564-
// Per #142 spec: ≥ 20 newlines triggers an inline confirm modal so a
565-
// multi-command paste can't accidentally run the entire clipboard.
566-
// Bracketed paste mode (DECSET 2004) is enabled by the remote shell;
564+
// Per #142 spec: ≥ 20 newline characters in clipboard text triggers
565+
// an inline confirm modal so a multi-command paste can't accidentally
566+
// run the entire clipboard. We count actual '\n' occurrences (not
567+
// lines) so a trailing newline doesn't shift the threshold off by one.
568+
// Bracketed paste mode (DECSET 2004) is the remote shell's job;
567569
// xterm wraps pasted content with ESC[200~ … ESC[201~ automatically
568-
// when the shell asked for it. This modal is the operator-side guard.
569-
const PASTE_CONFIRM_LINES = 20;
570+
// when the shell asked for it. Confirmed pastes route through
571+
// term.paste() so that wrapping still happens.
572+
const PASTE_CONFIRM_NEWLINES = 20;
570573

571574
ConsoleManager.prototype._wirePasteConfirm = function () {
572575
const modal = this._$('console-paste-modal');
@@ -589,7 +592,14 @@
589592
const text = self._pendingPaste;
590593
closeModal();
591594
const s = self._active();
592-
if (text && s) s.sendText(text);
595+
// Route through term.paste() so xterm applies its normal
596+
// paste pipeline (bracketed-paste wrapping, normalization).
597+
// sendText would bypass DECSET 2004 framing.
598+
if (text && s && s.term && typeof s.term.paste === 'function') {
599+
s.term.paste(text);
600+
} else if (text && s) {
601+
s.sendText(text);
602+
}
593603
});
594604

595605
// Listen at document level so paste lands here before xterm's
@@ -605,14 +615,21 @@
605615
if (!e.clipboardData) return;
606616
const text = e.clipboardData.getData('text');
607617
if (!text) return;
608-
const lines = text.split('\n').length;
609-
if (lines < PASTE_CONFIRM_LINES) return; // let xterm handle it normally
618+
// Count actual newline characters — robust against trailing-newline
619+
// quirks vs. counting lines via split().length.
620+
let newlines = 0;
621+
for (let i = 0; i < text.length; i++) {
622+
if (text.charCodeAt(i) === 10) newlines++;
623+
}
624+
if (newlines < PASTE_CONFIRM_NEWLINES) return; // let xterm handle it normally
610625
e.preventDefault();
611626
e.stopPropagation();
612627
self._pendingPaste = text;
613628
const linesEl = self._$('console-paste-modal-lines');
614629
const previewEl = self._$('console-paste-modal-preview');
615-
if (linesEl) linesEl.textContent = String(lines);
630+
// Visible "N lines" count = newlines + 1 (the line after the
631+
// final newline). Matches how the user thinks about it.
632+
if (linesEl) linesEl.textContent = String(newlines + 1);
616633
if (previewEl) {
617634
// Cap preview at ~1200 chars + first ~30 lines so a huge
618635
// paste doesn't blow up the modal. textContent is safe.
@@ -628,17 +645,23 @@
628645
/* ---------- global shortcut ---------- */
629646

630647
// Ctrl-Shift-` opens the console for the active box. Reads box_id from
631-
// the cookie set by switchBox; no cookie → no-op (palette is the
632-
// fallback path for users who haven't pinned a box yet).
648+
// the gearbox_active_box cookie set by the box-resolver middleware;
649+
// no cookie → no-op (palette is the fallback path for users who
650+
// haven't pinned a box yet).
651+
//
652+
// Registered eagerly on DOMContentLoaded — must not depend on the
653+
// user having opened the console at least once.
633654
ConsoleManager.prototype._wireGlobalShortcut = function () {
655+
if (this._globalShortcutWired) return;
656+
this._globalShortcutWired = true;
634657
const self = this;
635658
document.addEventListener('keydown', function (e) {
636659
if (!e.ctrlKey || !e.shiftKey || e.altKey || e.metaKey) return;
637660
// Match both literal backtick and "Backquote" code so Dvorak
638661
// and other non-QWERTY layouts work too.
639662
if (e.key !== '`' && e.code !== 'Backquote') return;
640663
e.preventDefault();
641-
const boxID = readCookie('box_id');
664+
const boxID = readCookie('gearbox_active_box');
642665
if (!boxID) return;
643666
self.open({ kind: 'box', boxID: boxID, label: boxID });
644667
});
@@ -1094,6 +1117,19 @@
10941117
manager: manager,
10951118
};
10961119

1120+
// Eagerly wire the global Ctrl-Shift-` shortcut so it works even
1121+
// before the user has opened the console once. Other wiring (dock
1122+
// resize, search bar, paste modal) needs the drawer markup present
1123+
// and stays inside the lazy _wire() path.
1124+
function eagerWire() {
1125+
try { manager._wireGlobalShortcut(); } catch (_) {}
1126+
}
1127+
if (document.readyState === 'loading') {
1128+
document.addEventListener('DOMContentLoaded', eagerWire);
1129+
} else {
1130+
eagerWire();
1131+
}
1132+
10971133
window.gearbox.console.markPopout = function () {
10981134
manager.popoutMode = true;
10991135
manager.layout = 'drawer';

0 commit comments

Comments
 (0)