In-TUI editor for comments/replies (no more $EDITOR handoff for short text) - #7
Merged
Merged
Conversation
Headless-testable Dialog + editable field (multi-line TextView or single-line TextField) implementing the ADR 0020 in-TUI text entry: Enter submits, Ctrl+J inserts a newline (probed: the dotnet/ansi driver delivers Ctrl+J as Enter|CtrlMask, not J|CtrlMask; Ctrl/Shift+Enter aren't distinctly representable so Ctrl+J is the fallback), Esc cancels, Ctrl+E hands the buffer to $EDITOR and returns to the field. Built without a run loop (mirrors TextDialog) and driven headlessly via NewKeyDownEvent.
Marshals the TextInputDialog build + modal app.Run onto the UI thread via app.Invoke and completes a RunContinuationsAsynchronously TCS, so background flows can await short text entry. The resolve callback captures the value and requests stop; Ctrl+E wires EditorService.EditAsync as the $EDITOR hatch. Also drops an unused field from TextInputDialog.
Comment and assign now read via the ADR 0020 in-TUI text-entry seam instead of the $EDITOR handoff. WorkItemActions gains an optional ITextInput dependency, defaulting to EditorTextInput(editor) as a behaviour-preserving compatibility shim for the not-yet-migrated WorkItemDetailDialog call site. Tags and description stay on $EDITOR (ADR 0020 scope).
Line comments, thread replies, PR replies, PR-level comments, and the resolve/reactivate/reply thread-id prompt now read via the ADR 0020 in-TUI text-entry seam instead of the $EDITOR handoff. ThreadViewDialog, DiffReviewDialog, and PrDetailDialog each take ITextInput in place of EditorService (no other flow in these three needed $EDITOR directly). CobaltShell wires a single EditorTextInput(editor) placeholder into all three plus WorkItemActions — a behaviour-preserving stand-in until the in-TUI TuiTextInput widget (built in parallel) is swapped in.
…lose Review-fix-loop (Codex + Fable) on the in-TUI editor: - HIGH: assign with an empty field no longer unassigns the item (empty submit is a dismiss, like the old $EDITOR no-op) — guard IsNullOrWhiteSpace. - HIGH: a failed Ctrl-E $EDITOR launch now surfaces in the dialog hint instead of silently refocusing; the fire-and-forget task observes all exceptions. - MEDIUM: close/resolve is once-only (a _closed guard) so queued keys can't double-submit and Ctrl-E post-backs can't touch the field after close; the editor call is cancelled on close so a queued suspend can't open over the shell. - Doc: note the Win32-driver multi-line-paste-submits-early limitation (ADR 0020).
Rebasing onto main merged its RefreshBody feature (ThreadViewDialog now subscribes vm.Changed → app.Invoke) into the editor branch's ITextInput migration. Two headless-test consequences: - The RefreshBody_* tests (main-side) passed the old EditorService via a NoopEditor() helper that no longer exists post-migration; switch them to the file's own FakeTextInput(null) no-op for the new ITextInput ctor. - The reply success test can no longer assert the post synchronously: RunThreadMutationAsync fires Changed (→ app.Invoke) before the store call and this suite never Application.Init()s. Assert the request only, mirroring the identical documented pattern in DiffReviewDialogKeyTests; the posted-text behavior stays covered by WorkItemActionsTests.
jinyeow
force-pushed
the
feature/inline-editor
branch
from
July 11, 2026 14:20
76e7e39 to
c847b69
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /// <param name="ct">Flows into the Ctrl+E editor call.</param> | ||
| /// <param name="post">Marshals the Ctrl+E result back onto the UI thread; defaults to | ||
| /// <see cref="IApplication.Invoke(Action)"/>. Injectable so a test observes the refill synchronously.</param> | ||
| public TextInputDialog( |
…er key edges Deep-review fixes on the in-TUI text field (ADR 0020): - Ctrl+E in-flight guard (_editorOpen): the suspend/resume handoff is deferred via the UI-thread marshal, so a double-press in the pre-park window queued a second $EDITOR launch over the same buffer (last-write-wins clobber). Guard blocks concurrent re-entry only; sequential Ctrl+E->edit->return->Ctrl+E still escalates. Reproduced RED (two launches) then GREEN in the headless harness. - Collapse the two byte-identical newline-chord branches into one guard. - Characterization tests: empty-Enter submits "" (not null), Shift+Enter newline arm, single-line Ctrl+Enter submit / Ctrl+J no-op guard branch.
Comment on lines
+174
to
+176
| if (!_request.SingleLine && | ||
| ((baseCode == KeyCode.Enter && (key.IsCtrl || key.IsShift)) || | ||
| (baseCode == KeyCode.J && key.IsCtrl))) |
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
2 participants
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.
Types comments and replies in a fast in-TUI editor instead of handing the terminal to
$EDITOR. Stacked on PR #4 (basefeature/diff-view-enhancements); sibling to the theming PR (#6). Retarget tomainafter #4 merges.Why
The
$EDITORsuspend/resume handoff (ADR 0009) is the root of a bug class on Windows Terminal — neovim's slow terminal-capability start (E1568), driver quirks, blank screens — and can only be UAT'd, never unit-tested. For a one-line comment (the thread-id prompt literally launched an editor to type a number) that's absurd. lazygit-style hybrid: built-in widget for short text,$EDITORfor long-form + on demand.What (ADR 0020)
ITextInput.ReadAsync(TextInputRequest) → string?seam;TextInputDialog(editableTextView, or single-lineTextField) +TuiTextInput. Headless-testable viaNewKeyDownEvent.$EDITORand returns for review.$EDITOR: descriptions, tags.How it was built
Contract-lock → 2 parallel lanes (widget / migration) → integrate + wire → review-fix-loop (Codex + Fable). 691 tests, 0 warnings.
Review-fix-loop findings (all fixed)
Both reviewers independently cleared the deadlock risk (Ctrl-E suspends TG from inside a nested modal loop — decompiled proof it pumps correctly). Fixed:
assignon an empty field no longer unassigns the item (empty submit = dismiss).Ctrl-E$EDITORlaunch now surfaces in the dialog hint (was a silent no-op).Notes / needs UAT
windowsdriver (no bracketed paste) may submit at the first newline — use Ctrl-E for that.Ctrl-Esuspend/resume round-trip in a real terminal.