clean lib/src/terminal/ to the project bar (T-107)
test / unit + widget + golden + a11y (push) Failing after 32s
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / integration_test (xvfb) (push) Has been skipped
test / dart doc (lib API) (push) Failing after 1m1s
test / unit + widget + golden + a11y (push) Failing after 32s
test / bundle smoke (xvfb 5s) (push) Has been skipped
test / daemon subprocess + web WASM smoke (push) Has been skipped
test / integration_test (xvfb) (push) Has been skipped
test / dart doc (lib API) (push) Failing after 1m1s
T-107 (b): treat the in-tree terminal as ours, not vendored. * custom_text_edit.dart — drop the row of commented-out `// print(...)` debugging stubs that shipped with the fork. * parser.dart — the "TODO: G2/G3" lines for unimplemented VT220 charset designators become a clear "not implemented" note; the stale "TODO: Normal/Application Keypad" tags on `>` / `=` get removed since the handlers ARE wired. * keytab.dart — the bare "TODO: support VT52" turns into a comment explaining that ANSI=false records are intentionally skipped (no clide consumer asks for VT52). * terminal_view.dart — the lone `// ignore: invalid_use_of_protected_member` keeps the suppression but gets an inline justification per CLAUDE.md (TerminalView owns its own ShortcutManager so terminal keybindings fire before the app's Shortcuts ancestor; wrapping in Shortcuts would invert that). parser.dart's 1139-LOC size is parked as T-123 — split is too invasive to fold here without conflicting with T-91's coverage sweep on the same area. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1817,3 +1817,4 @@ INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by,
|
||||
INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-115', 'status', 'backlog', 'in_progress', NULL, '2026-05-18 08:23:34', '2026-05-18 08:23:34', '2026-05-18 08:23:34', NULL, '94f4a894c1dddfa87ee8306a9758c808', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-115', 'status', 'in_progress', 'done', NULL, '2026-05-18 08:26:05', '2026-05-18 08:26:05', '2026-05-18 08:26:05', NULL, 'b8978359fc1b83ea0bcfc6cd5d34d5de', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-115', 'status', 'done', 'done', NULL, '2026-05-18 09:30:59', '2026-05-18 09:30:59', '2026-05-18 09:30:59', NULL, '2039c525646caaf89a735454f7278162', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-107', 'status', 'backlog', 'in_progress', NULL, '2026-05-18 10:27:25', '2026-05-18 10:27:25', '2026-05-18 10:27:25', NULL, 'e40d2c9ddd9915ad2e32302bd555c99f', 1) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -1944,3 +1944,28 @@ INSERT INTO tickets (id, type, parent_id, title, description, status, priority,
|
||||
**Out of scope:** multi-project workspaces, project groups, recent-project ordering changes.
|
||||
|
||||
**Source:** user request 2026-05-17.', 'done', 'medium', NULL, NULL, NULL, '2026-05-17 19:10:21', '2026-05-18 09:30:59', NULL, 'c88ab1b158beddb3cd74ad9a287bd224', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-107', 'task', 'T-97', 'decide lib/src/terminal/ status: formally vendor or clean to project bar', '~7k LOC forked from xterm.dart in an undeclared middle state. Carries commented-out `print()` debugging (`custom_text_edit.dart:244-275`), dangling TODOs (`parser.dart:110-113`, `keytab.dart:91`), a 1137-line `parser.dart`, and the only `// ignore: invalid_use_of_protected_member` in the repo (`terminal_view.dart:363`). Memory says "code under `lib/` is owned, not vendored."
|
||||
|
||||
**Decide one path and execute:**
|
||||
- **(a)** Formally vendor: relocate to `native/` or document as frozen in a D-record + `licenses.yaml`. Coverage carve-out documented.
|
||||
- **(b)** Clean to the project bar: remove debug prints, resolve TODOs, justify or remove the protected-member suppression, consider splitting `parser.dart`.
|
||||
|
||||
Source: consultants.md "Code quality — Findings — [Major]".', 'in_progress', 'medium', NULL, NULL, NULL, '2026-05-17 18:47:55', '2026-05-18 10:27:25', NULL, 'c6716898300053ff1a611a5e186c03fa', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
INSERT INTO tickets (id, type, parent_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('T-123', 'task', 'T-91', 'split lib/src/terminal/.../escape/parser.dart (1139 LOC)', 'parser.dart is a single 1139-line file containing the full ESC/CSI/OSC/DCS handler tree for the terminal emulator. Functional but unwieldy; the consultant flagged it as ''consider splitting'' in the T-107 review.
|
||||
|
||||
Suggested split (sequenced with the T-91 coverage sweep on lib/src/terminal/, so the split doesn''t fight in-flight test work):
|
||||
|
||||
- parser.dart — entry point + state machine driver
|
||||
- esc_handlers.dart — single-char ESC dispatch table + handlers
|
||||
- csi_handlers.dart — CSI parameter parsing + handlers
|
||||
- osc_handlers.dart — OSC string handlers (title, colour set, etc.)
|
||||
- dcs_handlers.dart — DCS/SOS/PM/APC string handlers
|
||||
|
||||
Each handler module exports a registrar that the driver wires at construction.
|
||||
|
||||
Done when:
|
||||
- parser.dart < 400 LOC
|
||||
- All existing parser tests pass without changes
|
||||
- No new public surface; everything stays library-private
|
||||
|
||||
Source: T-107 / consultants.md "Code quality — Findings — [Major]".', 'backlog', 'low', NULL, NULL, NULL, '2026-05-18 10:29:02', '2026-05-18 10:29:02', NULL, '1ac51fb8f92a9e72903c16e31f076955', 1) ON CONFLICT(id) DO UPDATE SET type=excluded.type, parent_id=excluded.parent_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash);
|
||||
|
||||
@@ -97,6 +97,14 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit.
|
||||
|
||||
### Changed
|
||||
|
||||
- `lib/src/terminal/` cleaned to the project bar — commented-out
|
||||
`print()` debugging stubs stripped from `custom_text_edit.dart`,
|
||||
stale TODOs in `parser.dart` + `keytab.dart` replaced with clear
|
||||
"not implemented" notes (G2/G3 charsets, VT52 records), and the
|
||||
one `// ignore: invalid_use_of_protected_member` in
|
||||
`terminal_view.dart` gets an inline reason explaining why
|
||||
TerminalView owns its own ShortcutManager. Parser split deferred
|
||||
to T-123 (T-107).
|
||||
- Window-control close-button red, white close glyph, and palette
|
||||
ambient shadow are now tokens (`windowControl.closeHover*`,
|
||||
`shadow.ambient`) instead of hard-coded hex. Light themes get a
|
||||
|
||||
@@ -105,12 +105,14 @@ class EscapeParser {
|
||||
// 'P'.charCode: _unsupportedHandler, // Sixel
|
||||
// 'c'.charCode: _unsupportedHandler,
|
||||
// '#'.charCode: _unsupportedHandler,
|
||||
'('.charCode: _escHandleDesignateCharset0, // SCS - G0
|
||||
')'.charCode: _escHandleDesignateCharset1, // SCS - G1
|
||||
// '*'.charCode: _voidHandler(1), // TODO: G2 (vt220)
|
||||
// '+'.charCode: _voidHandler(1), // TODO: G3 (vt220)
|
||||
'>'.charCode: _escHandleResetAppKeypadMode, // TODO: Normal Keypad
|
||||
'='.charCode: _escHandleSetAppKeypadMode, // TODO: Application Keypad
|
||||
'('.charCode: _escHandleDesignateCharset0, // SCS — G0
|
||||
')'.charCode: _escHandleDesignateCharset1, // SCS — G1
|
||||
// G2 (`ESC *`) and G3 (`ESC +`) charset designators are VT220+
|
||||
// sequences we don't honour — no consumer in clide selects past
|
||||
// G0/G1. Sequences pass through as no-ops (one trailing byte is
|
||||
// consumed by the default parser).
|
||||
'>'.charCode: _escHandleResetAppKeypadMode,
|
||||
'='.charCode: _escHandleSetAppKeypadMode,
|
||||
});
|
||||
|
||||
/// `ESC 7` Save Cursor (DECSC)
|
||||
|
||||
@@ -88,7 +88,9 @@ class Keytab {
|
||||
continue;
|
||||
}
|
||||
|
||||
// TODO: support VT52
|
||||
// VT52 mode (ANSI=false) records are skipped — clide only emits
|
||||
// ANSI/VT100+ sequences. No consumer asks for VT52 today; if one
|
||||
// does, this branch is where the lookup would flip.
|
||||
if (record.ansi == false) {
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -360,6 +360,13 @@ class TerminalViewState extends State<TerminalView> {
|
||||
return resultOverride;
|
||||
}
|
||||
|
||||
// ShortcutManager.handleKeypress is @protected — it's only public
|
||||
// for the Shortcuts widget to call internally. We own our own
|
||||
// ShortcutManager because the terminal has its own keybinding
|
||||
// surface (defaultTerminalShortcuts: Ctrl+C → kill, Tab handoff
|
||||
// rules, etc.) that needs to fire BEFORE the app's Shortcuts
|
||||
// ancestor. Wrapping in a Shortcuts widget would invert that.
|
||||
// T-107 approved leaving this suppression with an inline reason.
|
||||
// ignore: invalid_use_of_protected_member
|
||||
final shortcutResult = _shortcutManager.handleKeypress(
|
||||
focusNode.context!,
|
||||
|
||||
@@ -241,42 +241,27 @@ class CustomTextEditState extends State<CustomTextEdit> with TextInputClient {
|
||||
|
||||
@override
|
||||
void performAction(TextInputAction action) {
|
||||
// print('performAction $action');
|
||||
widget.onAction(action);
|
||||
}
|
||||
|
||||
@override
|
||||
void updateFloatingCursor(RawFloatingCursorPoint point) {
|
||||
// print('updateFloatingCursor $point');
|
||||
}
|
||||
void updateFloatingCursor(RawFloatingCursorPoint point) {}
|
||||
|
||||
@override
|
||||
void showAutocorrectionPromptRect(int start, int end) {
|
||||
// print('showAutocorrectionPromptRect');
|
||||
}
|
||||
void showAutocorrectionPromptRect(int start, int end) {}
|
||||
|
||||
@override
|
||||
void connectionClosed() {
|
||||
// print('connectionClosed');
|
||||
}
|
||||
void connectionClosed() {}
|
||||
|
||||
@override
|
||||
void performPrivateCommand(String action, Map<String, dynamic> data) {
|
||||
// print('performPrivateCommand $action');
|
||||
}
|
||||
void performPrivateCommand(String action, Map<String, dynamic> data) {}
|
||||
|
||||
@override
|
||||
void insertTextPlaceholder(Size size) {
|
||||
// print('insertTextPlaceholder');
|
||||
}
|
||||
void insertTextPlaceholder(Size size) {}
|
||||
|
||||
@override
|
||||
void removeTextPlaceholder() {
|
||||
// print('removeTextPlaceholder');
|
||||
}
|
||||
void removeTextPlaceholder() {}
|
||||
|
||||
@override
|
||||
void showToolbar() {
|
||||
// print('showToolbar');
|
||||
}
|
||||
void showToolbar() {}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user