Implements vertical multi-caret support#133
Merged
Merged
Conversation
PR #125 ships the right user-facing feature set (Alt+Up/Down for vertically-aligned carets, Alt+Drag for column-of-carets, Tab at all carets, normalization on primary-caret moves, Ctrl+Click after vertical block) but the implementation accreted around maintainer-reported regressions and is rejected as throwaway. This spec captures the behavior from the PR's tests, lifts the failing-first regressions as acceptance criteria, and flags the open caret-disappears-after-exit bug as OD-1 to be reproduced before the spec moves to Ready.
Cross-walk every user-facing behavior in the spec against VS Code and VS 2022/2026. Call out the three intentional divergences (Alt vs Ctrl+Alt vs Alt+Shift for add-caret-above/below; Alt+drag carets-only vs Shift+Alt+drag column-select; Ctrl+Click vs Alt+Click for caret-at-click) with rationale. Add OD-4 (keybinding choice) and OD-5 (Alt+Click alias).
Resolve OD-4 in favor of matching VS Code chords (modulo terminal incompat): Ctrl+Alt+Up/Down for add-caret-above/below, Shift+Alt+drag modifier for the column-drag. Comparison table rows that previously diverged on chords now match. The Shift+Alt+drag-produces-carets-only divergence stays (column-select-per-row is the follow-up); Ctrl+Click vs Alt+Click stays as a separate open question. Rename requirements from cryptic FR-### identifiers to human-readable labels (e.g. "Add caret above", "Caret normalization", "Cache invalidation on offset shift"). Open Decisions and scenarios are referenced by name throughout. Tests renamed from AltDown_* / AltDrag_* to CtrlAltDown_* / ShiftAltDrag_*. Add Resolved Decisions section capturing the keybinding choice and a new open question for whether ted should ship an alternate-chord fallback for terminals/WMs that grab Ctrl+Alt+arrow.
Resolve the alternate-chord open question: no editor-specific fallback. Keys are fully adjustable via TG keybindings, so pre-ship per-platform defaults the TG-standard way — a [ConfigurationProperty] DefaultKeyBindings populated from a PlatformKeyBinding record (All/Windows/Linux/Macos), user-overridable through View.ViewKeyBindings config. Windows/Linux default to the VS Code chord (Ctrl+Alt+Up/Down); the macOS default is a wiring-time empirical question (Terminal.app / iTerm2 swallow Cmd and may not deliver Ctrl+Alt+arrow) tracked as a remaining open decision. References TG docfx/docs/keyboard.md for the binding-layer model and Bind.AllPlus() helper.
Implements specs/vertical-multi-caret/spec.md — the re-spec of the rejected prototype PR #125, closing issue #124. Behaviour (VS Code chords, per DEC-006): - Ctrl+Alt+CursorUp / Ctrl+Alt+CursorDown add a caret above the topmost / below the bottommost caret at the sticky visual column (tabs, short lines, and word-wrap aware). Crossing a document bound is a no-op. - Shift+Alt + LeftButton drag builds a vertical column of carets (carets only; per-row selection is the documented follow-up). - Tab / Shift+Tab insert at every caret in one undo scope, each caret computing its own tab stop so the column stays aligned across presses. Design (addresses the maintainer's "hacky" rejection of #125): - Keybindings registered as Editor-local Command ids via AddCommand and bound through the configurable [ConfigurationProperty] Editor.DefaultKeyBindings (PlatformKeyBinding) — no inline if-chain in OnKeyDownNotHandled, no fallback chord. - AddAdditionalCaretAt (the only add path) dedupes by construction; NormalizeAdditionalCarets (the only invariant-trim path) runs on every primary move and document change; ToggleCaretAt rewritten on top. - Editor.Mouse.cs replaces the three suppress-until-release booleans with a single DragMode state; Ctrl-modified drags can no longer hijack the primary caret (Ctrl+Click reorder defence). - Visual-line cache: a same-line-count edit that shifts absolute offsets now invalidates downstream cached lines (lineDelta == 0 && offsetDelta != 0) — fixes the "Tab twice desyncs" defect; pinned by a unit test in EditorVisualLineCacheTests. - Vertical placement reuses the single-caret visual-column / wrap-map primitives (GetVisualColumnForOffset, TryGetVerticalOffset). Tests: 12 integration tests ported from the PR #125 set, re-keyed to the VS Code chords (failing-first on develop, green now) + 1 cache unit test. Full suites green (Tests 437/437, IntegrationTests 187/187); dotnet format + jb cleanup clean. OD-1 (primary caret disappears after exit) confirmed not reproducible on develop; kept Primary_Caret_Is_Visible_After_Exiting_MultiCaret as a regression guard. OD-3 resolved: ClearAdditionalCarets stays public (DEC-007). Docs (decisions, public-api, ted help) updated; stray CDATA wrapper stripped from the spec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Windows Terminal (and the xterm family it emulates) reserves Shift+drag as the user's forced text-selection override while an app has mouse mode on, and Alt turns that into a rectangular/block selection. So the spec's Shift+Alt+drag column gesture was being consumed by the terminal's own block-select and never reaching the editor. Alt+drag is forwarded. - Editor.Mouse.cs: column-drag press now keys on `alt` alone (was `shift && alt`); DragMode.ColumnCarets doc updated. - EditorMouseTests: ShiftAltDrag_Adds_Vertically_Aligned_Carets renamed to AltDrag_Adds_Vertically_Aligned_Carets; press/drag flags drop Shift. - Spec: authoritative Amendment block + normative refs updated to Alt; DEC-006 amended; public-api / Docs/Help (keyboard-reference, multi-caret) updated. VS-Code-describing text left accurate. Keyboard chords (Ctrl+Alt+Up/Down) unchanged. Making the mouse modifier user-configurable (to opt back into Shift+Alt parity) is tracked upstream by gui-cs/Terminal.Gui#4888 — to be prioritized. Full suites green: Tests 437/437, IntegrationTests 187/187; dotnet format + jb cleanup clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c90ea243d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review (P1) caught that the multi-caret path only covered Tab:
Unindent() still computed removals from the primary caret's line only,
so Shift+Tab in a vertical multi-caret session unindented one line and
left the other carets untouched — breaking the indent/unindent symmetry
the spec requires ("Tab and Shift+Tab honor HasMultipleCarets"; Files in
Scope names MultiCaretUnindent).
- Editor.MultiCaret.cs: add MultiCaretUnindent() — removes one
indentation unit from every distinct caret line (deduped), high-offset
-first, in a single RunUpdate scope; reuses
TextUtilities.GetSingleIndentationSegment like single-caret Unindent.
- Editor.Indentation.cs: Unindent() routes to MultiCaretUnindent() when
HasMultipleCarets (symmetric with InsertTab).
- Test: ShiftTab_Unindents_At_All_Carets_In_One_Undo_Step — confirmed
failing-first ("ab\n\tab\n\tab"), green after the fix; also asserts a
single Ctrl+Z restores all lines (one undo step).
Full suites green: Tests 437/437, IntegrationTests 188/188; dotnet
format + jb cleanup clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
tig
changed the title
7 tasks
Member
Author
|
Scope note: column / box selection (Alt+drag producing a selection per row) is intentionally deferred — this PR ships carets-only, per the spec's Out of Scope / Intentional Divergences #1 and DEC-006. Architectural review confirmed deferral is additive (nothing here needs undoing). Tracked, with the concrete code seams and the pre-existing additional-caret-selection rendering gap, in #139. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/ea1f9ebe-4830-4aee-b1f7-be12719a852c Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c286837bbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/ea1f9ebe-4830-4aee-b1f7-be12719a852c Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/ea1f9ebe-4830-4aee-b1f7-be12719a852c Co-authored-by: tig <585482+tig@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
This comment was marked as resolved.
This comment was marked as resolved.
Per maintainer review (bespoke (Command)1001/1002 casts are a no-no; the correct fix is upstream in TG's Command enum), filed gui-cs/Terminal.Gui#5318 requesting proper Command.InsertCaretAbove / .InsertCaretBelow members. - Editor.Commands.cs: comment above the const ids now marks them an explicitly TEMPORARY workaround tracked by TG#5318, with the removal plan (replace with real enum members + drop the block once TG#5318 ships and $(TerminalGuiVersion) is bumped). "Bespoke command values are not an acceptable end state." - specs/decisions.md DEC-006: same, cross-linked to TG#5318. Comment/doc only — no code-behavior change. Pure Tests suite green (454/454) on the merged canonical state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TG#5318 shipped Command.InsertCaretAbove / .InsertCaretBelow (TG PR
#5319, in Terminal.Gui 2.1.1-develop.98). Remove the sanctioned
short-term magic-int workaround now that the upstream fix is available:
- Directory.Build.props: pin $(TerminalGuiVersion) 2.1.1-develop.59 →
2.1.1-develop.98 (the develop build containing #5319).
- Editor.Commands.cs: delete the TEMPORARY WORKAROUND comment block and
the (Command)1001/1002 const casts; bind/AddCommand against the real
Command.InsertCaretAbove / Command.InsertCaretBelow members.
- specs/decisions.md DEC-006: mark the Command-enum debt RESOLVED
(2026-05-17), cross-link TG#5319 and the parked design issue TG#5320.
Keybindings now round-trip through config by readable name
("InsertCaretAbove") instead of the bare number "1001", satisfying the
gui-cs/Editor side of TG#5318's acceptance criterion.
Verified on 2.1.1-develop.98: restore + build clean (0 warnings),
Tests 454/454, IntegrationTests 207/207. `dotnet format
--verify-no-changes` clean. (jb cleanupcode hit the known local
profile-discovery flakiness the Stop hook documents; no style drift —
the edit is mechanical and dotnet format is the deterministic gate.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29200ed1bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Underline rendered poorly and inconsistently across terminals for the multi-caret indicator. Switch the MultiCaretRenderer caret attribute to TextStyle.Blink | TextStyle.Reverse — far more legible and reliably supported — and bring everything that described the old style into line: - MultiCaretRenderer.cs: rewrite the now-contradictory rationale comment and the XML-doc summary (was "underlined + blinking"). - EditorRenderingTests / EditorMouseTests: update the expected caret Attribute, the positive HasFlag assertion to Blink|Reverse, the negative assertions to "NOT Reverse", and rename MultiCaret_Renders_Underline_Blink_Attribute_On_Text → ..._Reverse_Blink_... . - Docs/Help/multi-caret.md, selection.md: "inverted-attribute cells" → "blinking, reverse-video cells"; also corrected the renderer interface name (IBackgroundRenderer → the actual IOverlayRenderer). Tests 454/454, IntegrationTests 207/207, dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings in develop: code-themes #134/#132 (xshd colorizer wired through TG Scheme code-token VisualRoles), TG pin .95→.98, Editor.MultiCaret.cs whitespace fix, HighlightingColor Role equality. Conflict: tests/.../EditorRenderingTests.cs — resolved by keeping our renamed MultiCaret_Renders_Reverse_Blink_Attribute_On_Text (the reverse-video caret change develop never had) and DROPPING the orphaned UseThemeBackground_Defaults_To_True test: develop removed the UseThemeBackground property in 04315ea ([#132] colorizer rework) with no replacement boolean, so the test referenced dead surface (it only survived on this branch via the earlier intermediate develop merge). Build clean; Tests 460/460; IntegrationTests 205/205; dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7685378b4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…odex P1) InsertTab()/Unindent() short-circuited to the multi-caret path the moment HasMultipleCarets was true — before the multi-line-selection branch — so a multi-line selection coexisting with an extra caret was silently REPLACED by a single tab (data loss) on Tab, and on Shift+Tab only the caret-hosting lines unindented (the block was ignored). Fix in Editor.MultiCaret.cs: classify each caret (primary uses the editor selection; additional uses its own anchor) via the new TryGetCaretSelectionRange / RangeSpansMultipleLines / LinesInRange helpers. A multi-line selection block-indents / block-unindents every line it touches; a single-line selection is type-over-replaced; a point caret gets a tab / line-unindent. All edits dedupe block lines and apply strictly high-offset-first in one RunUpdate scope (one undo step). A multi-line selection is never replaced/deleted by a tab. - specs/vertical-multi-caret/spec.md: amended the "Tab at every caret" requirement (was under-specified — "replacement, if a per-caret selection is active"), added the "Tab with a multi-line selection plus an extra caret" scenario, recorded the Resolved Decision. - New EditorMultiCaretIndentTests: Tab and Shift+Tab with a multi-line primary selection + a point caret; assert block-indent (not delete) and single-step undo. These failed before the fix. Tests 460/460; IntegrationTests 207/207; dotnet format clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tig
added a commit
that referenced
this pull request
May 17, 2026
Loose end carried forward from PR #133 (now merged): multi-caret Tab/Shift+Tab block-indent calls ClearAdditionalCaretSelections() and collapses the primary selection post-edit, whereas single-caret IndentSelectedLines preserves it. Cosmetic while carets are point-only, but load-bearing once per-row selections become a first-class column primitive — so it belongs to the column/box selection-per-row ("multi-select") follow-up. - specs/vertical-multi-caret/spec.md: the Out-of-Scope "Column / box selection" bullet now names it "the multi-select PR" and lists selection-preservation parity (primary + per-caret, mirroring SetSelectionRangePreservingDirection, with a regression test) as a REQUIRED deliverable of that PR. - specs/multi-caret/spec.md: Out of Scope cross-references the full requirement so it's discoverable from the canonical multi-caret spec. Spec/docs only — no code change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Key bindings
Ctrl+Alt+↑Ctrl+Alt+↓Alt+ LeftButton dragAlt, not VS Code'sShift+Alt— Windows Terminal eatsShift+drag for block-select; carets onlyCtrl+ClickPositionReport+Ctrlno longer hijacks the primaryTab/Shift+TabEsc↑/↓navigation is not trapped by where the block wasCtrl+Alt+↑/↓is a per-platform[ConfigurationProperty]PlatformKeyBindinginEditor.DefaultKeyBindings— fully user-overridable viaView.ViewKeyBindingsconfig,no editor-specific fallback chord (macOS same chord pending real-terminal validation,
DEC-006); registered via
AddCommand+ the configurable path, not an inlineif.The mouse column-drag uses
Alt(not VS Code'sShift+Alt): a TUI runs inside aterminal that reserves
Shift+drag for its own forced/block text selection, soShift+Alt+drag never reaches the editor. The mouse modifier is not yetuser-configurable — tracked upstream by
gui-cs/Terminal.Gui#4888 (configurable
MouseBindings), to be prioritized.Validation
on
develop, green after implementation — plus 1 visual-line-cache unit test.dotnet format+jbcleanup clean; 0 warnings.
develop— keptPrimary_Caret_Is_Visible_After_Exiting_MultiCaretas a regression guard.ClearAdditionalCaretsstayspublic(DEC-007).No new public
EditorAPI (R8). Docs updated:specs/decisions.md,specs/public-api.md,Docs/Help/keyboard-reference.md,Docs/Help/multi-caret.md.Original spec-PR description
Summary
Adds
specs/vertical-multi-caret/spec.md. No code changes.PR #125 ships the right user-facing feature set —
Alt+Up/Alt+Downto create vertically-aligned carets,Alt+Dragto create a column,Tabat every caret, normalization so the primary moving onto an additional caret doesn't duplicate edits, and Ctrl+Click after a vertical block landing where the user clicked — but the maintainer rejected it as hacky after a stream of regressions kept popping out under each fix:…and four follow-ups, ending with the unresolved "primary caret disappears after exiting multi mode" that Copilot couldn't reproduce.
This spec treats PR #125's tests as the executable contract and re-derives the requirements / file scope / definition of done from black-box behavior, not from the patch. The intended workflow is: open a new branch from
develop, port the PR #125 tests verbatim, confirm they fail, then implement against this spec.What the spec covers
ClearAdditionalCaretsshould drop tointernalper R9).What this PR is not
src/ortests/changes.developand rebuild against the test set.Test plan
develop) before moving the spec from Draft → Ready.https://claude.ai/code/session_014uBfGKbouM7qvzSShb3WCc
Generated by Claude Code