Uh oh!
There was an error while loading. Please reload this page.
The away divider: mark where the reader was when they left the terminal - #20
Conversation
A boundary row drawn where the reader left off when they tab away from the terminal, so returning does not mean guessing which lines are already read. The signal is terminal focus reporting (CSI ?1004h), which SharpConsoleUI neither asks for nor decodes. Both halves of the workaround are recorded: WriteClipboardOsc52 is a raw-escape writer wearing its first customer's name, and focus-in reaches us disguised as a Tab keypress because DispatchCsi reads a trailing I as Tab. Focus-out is discarded upstream and is not recoverable, so the boundary anchors to the last input event instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX
AwayBarRenderer is a sibling of the freeze and restore bars and carries the same two figures the restore bar does, for the same reason: a returning reader asks both how much is in front of them and how far behind they are. TerminalFocusWatcher is the whole workaround, in one file so it is one file to delete. It asks for focus reporting through WriteClipboardOsc52 — named for its first customer, but a verbatim raw write under the renderer's own lock — and recognises focus-in in the Tab keypress the framework's parser mistranslates it into. Telling that Tab from a real one is a question about time, and the comparison has to be against the input before it: the disguised focus-in is itself a KeyPressed, raised before the global shortcuts run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX
Tabbing away and coming back meant guessing which lines had already been read: every pane bottom-anchors through an absence, so what you land on is the newest output with no boundary in it, and the unread badges are silent about it — NoteActivity only counts a line while a window is *not* caught up, and the window you were looking at stays visible and at its tail the whole time. The boundary is tracked forward, on every input event, because it cannot be found afterwards: a PaneLine's stamp is formatted text, not a time. It is kept one input back, for the reason the watcher's clock is — the focus report is itself a keypress and has already moved the newer of the two by the time anything recognises it as a return. Consumption is not Workspace.IsCaughtUp. A bottom-anchored pane satisfies that the instant you return with two hundred unread lines above the fold. The bar goes when it has been inside the viewport, the pane is at its live tail, and one input has landed since it was drawn. Also fixes SimulateKey discarding a global shortcut's result: it swallowed the key whether or not the handler claimed it, which was invisible while every claim returned true and is not any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX
WalkthroughThis change adds Unix-only terminal focus tracking and per-window away dividers. It records reader boundaries, renders and consumes markers, adjusts state during scrollback trimming, updates snapshot scenarios, and forwards declined global shortcuts to normal key handling. ChangesAway divider flow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TerminalFocusWatcher
participant SharpMUTermApp
participant ConsoleWindow
participant AwayBarRenderer
TerminalFocusWatcher->>SharpMUTermApp: report focus return
SharpMUTermApp->>ConsoleWindow: save reader boundary
ConsoleWindow-->>SharpMUTermApp: expose missed output and viewport state
SharpMUTermApp->>AwayBarRenderer: format away marker
AwayBarRenderer-->>SharpMUTermApp: return AWAY bar
SharpMUTermApp->>ConsoleWindow: consume marker after viewing and input
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 232-237: Update the output-area documentation near the existing
“three scroll* views” statement to include away-scrollback as the fourth view
that can contain more output than a pane holds. Keep the surrounding behavior
descriptions unchanged and ensure the count and named views are consistent with
the away-scrollback entry.
In `@docs/superpowers/specs/2026-08-10-away-divider-design.md`:
- Line 4: Update the Status field in the away-divider design document to
indicate that the design is implemented rather than proposed, and correct the
sentence near the referenced lines by changing “through” to the grammatically
correct verb form “throughout.”
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs`:
- Around line 540-541: The focus event handlers in SharpMUTermApp must marshal
driver-thread callbacks onto the UI thread; update the _focus.Input and
_focus.Returned subscriptions to invoke NoteReaderInput and
MarkWhereTheReaderLeft through OnUiThread. Also update the TerminalFocusWatcher
class remark to state that Input runs on the thread delivering the driver event,
with mouse events arriving on the driver input thread.
- Around line 2456-2494: Update MarkWhereTheReaderLeft to retain RemoveAwayBar’s
return value and repaint the window when a previous away bar was removed,
including when missed <= 0. Preserve the existing bar insertion and repaint
behavior for windows that gained lines, and add the regression test beside
AWindowThatGainedNothingGetsNoBar covering a second no-output return with no
AwayBarRenderer.Label row remaining.
- Around line 4129-4139: Update RegisterFocusReportTab so its registered action
invokes TryTakeAsReturn first and calls DisarmPrefix only when that method
returns true. Preserve the existing declined-Tab behavior by returning false
without disarming, allowing HandleWindowKey and ConsumePrefixKey to handle it
normally.
- Around line 885-911: Move the ReArmWholeFrame call from the shared view path
into the deep branch after both PageUp simulations. Keep SettleScroll unchanged,
and ensure the away path does not receive the extra re-arm while away-scrollback
still does after scrolling back.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ceb2bee4-3030-4292-8f11-9c9100f56873
📒 Files selected for processing (9)
CLAUDE.mddocs/superpowers/specs/2026-08-10-away-divider-design.mdsrc/SharpMUTerm.Tui/AwayBarRenderer.cssrc/SharpMUTerm.Tui/Glyphs.cssrc/SharpMUTerm.Tui/SharpMUTermApp.cssrc/SharpMUTerm.Tui/TerminalFocusWatcher.cstests/SharpMUTerm.Tui.Tests/AwayBarRendererTests.cstests/SharpMUTerm.Tui.Tests/AwayDividerTests.cstests/SharpMUTerm.Tui.Tests/TerminalFocusWatcherTests.cs
| `rail-long`, `scrollback`, `scrollback-up`, `freeze-scrollback`, | ||
| `away`/`away-scrollback` (the bar marking where the reader was when they tabbed away from the | ||
| *terminal* — the shallow absence, where the bar and everything below it are on screen at once, and the | ||
| deep one, where more arrived than the pane holds and the reader has scrolled back to find it; the | ||
| second is the only frame that can show a bottom-anchored pane being "caught up" while nothing has been | ||
| read), `prefix-panel` (the ⌃B which-key |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The new away-scrollback view contradicts the "three scroll* views" claim below.
This entry states that the deep view has "more arrived than the pane holds". Line 274 still reads: "The three scroll* views are the only ones with more output than a pane holds."
away-scrollback feeds 40 lines into a 32-row frame, so that count is now four views, not three. The claim at line 274 is the one a reader consults when a change touches the output area, so it should name the new view.
📝 Proposed fix (line 274)
-- **The three `scroll*` views are the only ones with more output than a pane holds.** Every other view- fits, which is exactly why no snapshot caught the panes being unable to scroll at all. Reach for one- of these (or `LoadLongScene`) whenever a change touches the output area.+- **The three `scroll*` views and `away-scrollback` are the only ones with more output than a pane+ holds.** Every other view fits, which is exactly why no snapshot caught the panes being unable to+ scroll at all. Reach for one of these (or `LoadLongScene`) whenever a change touches the output area.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@CLAUDE.md` around lines 232 - 237, Update the output-area documentation near
the existing “three scroll* views” statement to include away-scrollback as the
fourth view that can contain more output than a pane holds. Keep the surrounding
behavior descriptions unchanged and ensure the count and named views are
consistent with the away-scrollback entry.
| # The away divider: where you were when you left the terminal | ||
| **Date:** 2026-08-10 | ||
| **Status:** proposed — design only, nothing implemented |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the status field and fix one sentence.
Line 4 states proposed — design only, nothing implemented. This PR implements the design in the same change set, so the status is already wrong at merge time.
Line 9 reads "Every pane has bottom-anchored through your absence". The verb form is wrong.
📝 Proposed fix
-**Status:** proposed — design only, nothing implemented+**Status:** implemented — `AwayBarRenderer`, `TerminalFocusWatcher`, `SharpMUTermApp` away-mark lifecycle-Tab away from the terminal, come back later, and there is no way to tell which of the lines on-screen you have already read. Every pane has bottom-anchored through your absence, so what you land-on is the newest output with no boundary in it. The client already knows how to say "the rows above+Tab away from the terminal, come back later, and there is no way to tell which of the lines on+screen you have already read. Every pane has bottom-anchored itself through your absence, so what you+land on is the newest output with no boundary in it. The client already knows how to say "the rows aboveAlso applies to: 8-9
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-08-10-away-divider-design.md` at line 4, Update
the Status field in the away-divider design document to indicate that the design
is implemented rather than proposed, and correct the sentence near the
referenced lines by changing “through” to the grammatically correct verb form
“throughout.”
| _focus.Input += NoteReaderInput; | ||
| _focus.Returned += MarkWhereTheReaderLeft; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
UI-thread state is mutated from the driver input thread.TerminalFocusWatcher subscribes to IConsoleDriver.MouseEvent and raises its own Input event inline from that handler. SharpMUTermApp subscribes UI-thread work to that event without marshalling. The shared root cause is one missing hop onto the UI thread between the watcher's driver callbacks and the app's away-marker state.
SharpMUTermApp documents the mouse stream as the input thread in three places: the _paneTabsLock remark at lines 164-169, the OnDriverMouseEvent comment at lines 670-673, and the OnTabCloseRequested remarks. Every other driver-level subscription in the file marshals through OnUiThread.
The reachable failures are an InvalidOperationException when NoteReaderInput enumerates _lines while AppendWindowLine adds a window key, and a pane re-feed through RepaintPane → FeedRange → MarkupControl.SetContent during a paint.
src/SharpMUTerm.Tui/SharpMUTermApp.cs#L540-L541: wrap both handlers inOnUiThread, so_focus.Input += () => OnUiThread(NoteReaderInput);and_focus.Returned += away => OnUiThread(() => MarkWhereTheReaderLeft(away));.src/SharpMUTerm.Tui/TerminalFocusWatcher.cs#L191-L213: correct the class remark at lines 84-88, which claims the events run on the UI thread. State thatInputis raised on whichever thread the driver delivered the event on, and that the mouse path is the driver input thread.
As per coding guidelines: "Marshal background work onto the SharpConsoleUI UI thread with system.EnqueueOnUIThread."
📍 Affects 2 files
src/SharpMUTerm.Tui/SharpMUTermApp.cs#L540-L541(this comment)src/SharpMUTerm.Tui/TerminalFocusWatcher.cs#L191-L213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 540 - 541, The focus
event handlers in SharpMUTermApp must marshal driver-thread callbacks onto the
UI thread; update the _focus.Input and _focus.Returned subscriptions to invoke
NoteReaderInput and MarkWhereTheReaderLeft through OnUiThread. Also update the
TerminalFocusWatcher class remark to state that Input runs on the thread
delivering the driver event, with mouse events arriving on the driver input
thread.
Source: Coding guidelines
| // The bar marking where the reader was when they tabbed away from the terminal. Two views, | ||
| // because the two states it has are the two ends of the consumption rule. `away` is a shallow | ||
| // absence: the bar and everything below it are on screen at once, which is the frame where the | ||
| // bar has to be legible without being loud. `away-scrollback` is a deep one, where more arrived | ||
| // than the pane can hold — the bar is above the fold, the reader has gone back to look for it, | ||
| // and the frame carries it with the lines it divides on both sides of it *and* the status row's | ||
| // scrollback segment. Only the second can show that a bottom-anchored pane being "caught up" | ||
| // says nothing about whether the lines have been read, which is the mistake the obvious | ||
| // consumption rule makes. | ||
| if (string.Equals(view, "away", StringComparison.OrdinalIgnoreCase) || | ||
| string.Equals(view, "away-scrollback", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| var deep = string.Equals(view, "away-scrollback", StringComparison.OrdinalIgnoreCase); | ||
| SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.End, false, false, false)); // the reader is here | ||
| LoadLongScene(MainWindowId, deep ? 40 : 4); | ||
| SimulateReturnFromAway(TimeSpan.FromMinutes(deep ? 143 : 12)); | ||
| SettleScroll(); | ||
| if (deep) | ||
| { | ||
| SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); | ||
| SimulateKey(new ConsoleKeyInfo('\0', ConsoleKey.PageUp, false, false, false)); | ||
| } | ||
| ReArmWholeFrame(); | ||
| } | ||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Description: Confirm the two new snapshot views exist and are wired into the documented view list.set -uo pipefail
# The view names must be reachable from RenderSnapshot and listed in CLAUDE.md.
rg -n '"away"|"away-scrollback"|away-scrollback' src/SharpMUTerm.Tui/SharpMUTermApp.cs CLAUDE.md
# The frame renderer helper the guideline points at, and the long-scene feeder these views use.
rg -nP -C3 'LoadLongScene|SimulateReturnFromAway|SettleScroll\(\)' src/SharpMUTerm.Tui/SharpMUTermApp.csRepository: SharpMUSH/SharpMUTerm
Length of output: 3204
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- RenderSnapshot away branch ---'
sed -n '840,930p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
printf'%s\n''--- SettleScroll and ReArmWholeFrame ---'
sed -n '1225,1275p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
printf'%s\n''--- snapshot entry points and invocation paths ---'
rg -n -C4 'RenderSnapshot|demo-config|snapshot|ReArmWholeFrame|RenderFrame\(' src tests .github README.md CLAUDE.md 2>/dev/null | head -n 300
printf'%s\n''--- project files and scripts ---'
git ls-files | rg '(^|/)(.*(test|snapshot|demo|script|README|CLAUDE).*)$|\.csproj$|\.sln'Repository: SharpMUSH/SharpMUTerm
Length of output: 47010
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- RenderSnapshot method boundaries and final render ---'
rg -n 'RenderSnapshot|RenderWholeFrame|RenderFrame|SimulateKey' src/SharpMUTerm.Tui/SharpMUTermApp.cs | head -n 80
sed -n '760,845p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
sed -n '930,980p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
sed -n '1320,1375p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
printf'%s\n''--- snapshot CLI implementation ---'
rg -n -C5 'Snapshot|snapshot|DemoConfig|demo-config|RenderSnapshot' src/SharpMUTerm.Tui/Program.cs src/SharpMUTerm.TuiRepository: SharpMUSH/SharpMUTerm
Length of output: 50378
🏁 Script executed:
#!/bin/bashset -euo pipefail
printf'%s\n''--- RenderSnapshot start and completion ---'
sed -n '730,755p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
sed -n '960,1015p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
printf'%s\n''--- SimulateKey behavior ---'
sed -n '6950,6995p' src/SharpMUTerm.Tui/SharpMUTermApp.cs
printf'%s\n''--- documented away views ---'
sed -n '225,240p' CLAUDE.mdRepository: SharpMUSH/SharpMUTerm
Length of output: 8572
Move ReArmWholeFrame into the deep branch.
SettleScroll already calls ReArmWholeFrame; retain the extra call only after the two PageUp calls. Render and inspect both away and away-scrollback with --demo-config.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 885 - 911, Move the
ReArmWholeFrame call from the shared view path into the deep branch after both
PageUp simulations. Keep SettleScroll unchanged, and ensure the away path does
not receive the extra re-arm while away-scrollback still does after scrolling
back.
Source: Coding guidelines
| private void MarkWhereTheReaderLeft(TimeSpan away) | ||
| { | ||
| var accent = FrozenAccentHex(); | ||
| foreach (var windowId in _lines.Keys.ToArray()) | ||
| { | ||
| if (string.Equals(windowId, WebWindowId, StringComparison.Ordinal)) | ||
| { | ||
| continue; | ||
| } | ||
| if (_freezePoints.TryGetValue(windowId, out var point)) | ||
| // At most one per window, so the previous bar goes first — and it goes first rather than | ||
| // last because removing it shifts every index after it, the pending boundary included. | ||
| RemoveAwayBar(windowId); | ||
| var buffer = _lines[windowId]; | ||
| var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count); | ||
| var missed = buffer.Count - at; | ||
| if (missed <= 0) | ||
| { | ||
| var split = Math.Clamp(point, 0, buffer.Count); | ||
| if (_frozenPanes.TryGetValue(windowId, out var frozen)) | ||
| { | ||
| FeedRange(frozen, buffer, 0, split); | ||
| } | ||
| continue; | ||
| } | ||
| if (_panes.TryGetValue(windowId, out var tail)) | ||
| { | ||
| FeedRange(tail, buffer, split, buffer.Count - split); | ||
| } | ||
| buffer.Insert(at, new PaneLine(AwayBarRenderer.Bar(missed, away, accent))); | ||
| if (_freezePoints.TryGetValue(windowId, out var freeze) && freeze > at) | ||
| { | ||
| _freezePoints[windowId] = freeze + 1; | ||
| } | ||
| _awayMarks[windowId] = new AwayMark { Index = at }; | ||
| RepaintPane(windowId); | ||
| } | ||
| // The reader is back and this is where they are now, so the next absence measures from here | ||
| // rather than from the keystroke before the last one. | ||
| foreach (var (windowId, buffer) in _lines) | ||
| { | ||
| _awayPending[windowId] = _awayBoundary[windowId] = buffer.Count; | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A removed previous bar is not repainted when the window gained nothing.
RemoveAwayBar at line 2468 takes the previous bar out of the line buffer. If missed <= 0 at line 2473, the loop then continues and never reaches RepaintPane at line 2485.
That path is reachable. It needs a window that still carries an unconsumed bar from an earlier absence and that received no lines since the reader's last input. RemoveAwayBar decrements _awayBoundary when the boundary sits past the removed row, so boundary == buffer.Count and missed == 0.
The row is then gone from _lines but still on the control. AppendWindowLine only calls control.AppendLine, so no later line prunes it. The stale AWAY row stays on screen until an unrelated RepaintPane, a freeze toggle, or a pane rebuild.
Use RemoveAwayBar's return value, which already reports whether a bar was removed.
🐛 Proposed fix
// At most one per window, so the previous bar goes first — and it goes first rather than
// last because removing it shifts every index after it, the pending boundary included.
- RemoveAwayBar(windowId);+ var replaced = RemoveAwayBar(windowId);
var buffer = _lines[windowId];
var at = Math.Clamp(_awayBoundary.GetValueOrDefault(windowId), 0, buffer.Count);
var missed = buffer.Count - at;
if (missed <= 0)
{
+ // Nothing arrived, so there is no boundary to mark — but a bar that was just taken+ // out of the buffer is still on the control until the pane is re-fed.+ if (replaced)+ {+ RepaintPane(windowId);+ }+
continue;
}A test belongs beside AWindowThatGainedNothingGetsNoBar in tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs: draw a bar, return again with no output in between, then assert that no row in PaneLines(Main) contains AwayBarRenderer.Label.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| privatevoidMarkWhereTheReaderLeft(TimeSpanaway) | |
| { | |
| varaccent=FrozenAccentHex(); | |
| foreach(varwindowIdin_lines.Keys.ToArray()) | |
| { | |
| if(string.Equals(windowId,WebWindowId,StringComparison.Ordinal)) | |
| { | |
| continue; | |
| } | |
| if(_freezePoints.TryGetValue(windowId,outvarpoint)) | |
| // At most one per window, so the previous bar goes first — and it goes first rather than | |
| // last because removing it shifts every index after it, the pending boundary included. | |
| RemoveAwayBar(windowId); | |
| varbuffer=_lines[windowId]; | |
| varat=Math.Clamp(_awayBoundary.GetValueOrDefault(windowId),0,buffer.Count); | |
| varmissed=buffer.Count-at; | |
| if(missed<=0) | |
| { | |
| varsplit=Math.Clamp(point,0,buffer.Count); | |
| if(_frozenPanes.TryGetValue(windowId,outvarfrozen)) | |
| { | |
| FeedRange(frozen,buffer,0,split); | |
| } | |
| continue; | |
| } | |
| if(_panes.TryGetValue(windowId,outvartail)) | |
| { | |
| FeedRange(tail,buffer,split,buffer.Count-split); | |
| } | |
| buffer.Insert(at,newPaneLine(AwayBarRenderer.Bar(missed,away,accent))); | |
| if(_freezePoints.TryGetValue(windowId,outvarfreeze)&&freeze>at) | |
| { | |
| _freezePoints[windowId]=freeze+1; | |
| } | |
| _awayMarks[windowId]=newAwayMark{Index=at}; | |
| RepaintPane(windowId); | |
| } | |
| // The reader is back and this is where they are now, so the next absence measures from here | |
| // rather than from the keystroke before the last one. | |
| foreach(var(windowId,buffer)in_lines) | |
| { | |
| _awayPending[windowId]=_awayBoundary[windowId]=buffer.Count; | |
| } | |
| } | |
| privatevoidMarkWhereTheReaderLeft(TimeSpanaway) | |
| { | |
| varaccent=FrozenAccentHex(); | |
| foreach(varwindowIdin_lines.Keys.ToArray()) | |
| { | |
| if(string.Equals(windowId,WebWindowId,StringComparison.Ordinal)) | |
| { | |
| continue; | |
| } | |
| // At most one per window, so the previous bar goes first — and it goes first rather than | |
| // last because removing it shifts every index after it, the pending boundary included. | |
| varreplaced=RemoveAwayBar(windowId); | |
| varbuffer=_lines[windowId]; | |
| varat=Math.Clamp(_awayBoundary.GetValueOrDefault(windowId),0,buffer.Count); | |
| varmissed=buffer.Count-at; | |
| if(missed<=0) | |
| { | |
| // Nothing arrived, so there is no boundary to mark — but a bar that was just taken | |
| // out of the buffer is still on the control until the pane is re-fed. | |
| if(replaced) | |
| { | |
| RepaintPane(windowId); | |
| } | |
| continue; | |
| } | |
| buffer.Insert(at,newPaneLine(AwayBarRenderer.Bar(missed,away,accent))); | |
| if(_freezePoints.TryGetValue(windowId,outvarfreeze)&&freeze>at) | |
| { | |
| _freezePoints[windowId]=freeze+1; | |
| } | |
| _awayMarks[windowId]=newAwayMark{Index=at}; | |
| RepaintPane(windowId); | |
| } | |
| // The reader is back and this is where they are now, so the next absence measures from here | |
| // rather than from the keystroke before the last one. | |
| foreach(var(windowId,buffer)in_lines) | |
| { | |
| _awayPending[windowId]=_awayBoundary[windowId]=buffer.Count; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 2456 - 2494, Update
MarkWhereTheReaderLeft to retain RemoveAwayBar’s return value and repaint the
window when a previous away bar was removed, including when missed <= 0.
Preserve the existing bar insertion and repaint behavior for windows that gained
lines, and add the regression test beside AWindowThatGainedNothingGetsNoBar
covering a second no-output return with no AwayBarRenderer.Label row remaining.
| private void RegisterFocusReportTab() | ||
| { | ||
| if (!_focus.IsEnabled) | ||
| { | ||
| return; | ||
| } | ||
| Func<bool> action = _focus.TryTakeAsReturn; | ||
| _system.RegisterGlobalShortcut((ConsoleModifiers)0, ConsoleKey.Tab, action); | ||
| _shortcuts[((ConsoleModifiers)0, ConsoleKey.Tab)] = action; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
A consumed focus-report Tab leaves an armed ⌃B prefix behind.
RegisterGlobalShortcuts wraps every claimed chord except ⌃B in a DisarmPrefix decorator at lines 4091-4095. The comment there states the reason: a global shortcut runs ahead of any window, so one pressed while ⌃B was pending left the prefix armed with nothing able to consume it, "and x closes a window".
RegisterFocusReportTab registers Tab without that decorator.
The declining case is safe. A declined Tab reaches HandleWindowKey, which sees _prefixArmed and routes it to ConsumePrefixKey, and that disarms.
The consuming case is not. TryTakeAsReturn returns true, the key never reaches the window, and _prefixArmed stays true. The next keystroke is then eaten as a pane command. The sequence is reachable: press ⌃B, leave the terminal for more than TerminalFocusWatcher.DefaultReturnThreshold, then return.
🐛 Proposed fix
- Func<bool> action = _focus.TryTakeAsReturn;+ // Same decorator every other claimed chord gets, and for the same reason: a global shortcut+ // runs ahead of any window, so a consumed Tab would leave ⌃B armed with nothing able to+ // spend it and the next key would be eaten as a pane command. A *declined* Tab reaches+ // HandleWindowKey and disarms there, so only the consuming arm needs this.+ Func<bool> action = () =>+ {+ if (!_focus.TryTakeAsReturn())+ {+ return false;+ }++ DisarmPrefix();+ return true;+ };
_system.RegisterGlobalShortcut((ConsoleModifiers)0, ConsoleKey.Tab, action);
_shortcuts[((ConsoleModifiers)0, ConsoleKey.Tab)] = action;Note the ordering: DisarmPrefix must run only after TryTakeAsReturn reports true, so a declined Tab still disarms through the normal window path rather than twice.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| privatevoidRegisterFocusReportTab() | |
| { | |
| if(!_focus.IsEnabled) | |
| { | |
| return; | |
| } | |
| Func<bool>action=_focus.TryTakeAsReturn; | |
| _system.RegisterGlobalShortcut((ConsoleModifiers)0,ConsoleKey.Tab,action); | |
| _shortcuts[((ConsoleModifiers)0,ConsoleKey.Tab)]=action; | |
| } | |
| privatevoidRegisterFocusReportTab() | |
| { | |
| if(!_focus.IsEnabled) | |
| { | |
| return; | |
| } | |
| // Same decorator every other claimed chord gets, and for the same reason: a global shortcut | |
| // runs ahead of any window, so a consumed Tab would leave ⌃B armed with nothing able to | |
| // spend it and the next key would be eaten as a pane command. A *declined* Tab reaches | |
| // HandleWindowKey and disarms there, so only the consuming arm needs this. | |
| Func<bool>action=()=> | |
| { | |
| if(!_focus.TryTakeAsReturn()) | |
| { | |
| returnfalse; | |
| } | |
| DisarmPrefix(); | |
| returntrue; | |
| }; | |
| _system.RegisterGlobalShortcut((ConsoleModifiers)0,ConsoleKey.Tab,action); | |
| _shortcuts[((ConsoleModifiers)0,ConsoleKey.Tab)]=action; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 4129 - 4139, Update
RegisterFocusReportTab so its registered action invokes TryTakeAsReturn first
and calls DisarmPrefix only when that method returns true. Preserve the existing
declined-Tab behavior by returning false without disarming, allowing
HandleWindowKey and ConsumePrefixKey to handle it normally.
… the fold Reported as "I saw no change". The mechanism was working — proven against a real kitty: the client emits ?1004h, a DECRQM probe answers ?1004;1 after the alternate-screen switch and every mode set behind it, the terminal writes ESC [ O and ESC [ I, and a 42-second absence was recognised. What was missing was any way to tell. Come back to more lines than the pane holds and the bar is drawn far above the viewport, so nothing on screen changes. Nothing else covers for it either: a window that is visible and at its live tail throughout an absence accrues no unread badge, because NoteActivity counts only what arrives while not caught up. The reader got no signal at all, which is indistinguishable from the terminal not reporting focus. RevealAwayBar scrolls each pane that gained a bar so the bar is at the top of its viewport. A bar already in view is left alone — scrolling a shallow absence would take a pane off its tail to reveal what is already on it. The first cut of that scroll was itself wrong, and the frame is what caught it: a buffer index is not a viewport row, a wrapped line occupies several, and in a narrow pane the scroll landed hundreds of rows adrift in content from a previous session. The tail height is measured through the framework's own MeasureDOM, so it wraps the way the real control will, and only the tail is measured. Consumption drops to two conjuncts. "At the live tail" now means something on its own, because the reveal has taken the pane off its tail whenever the bar was not on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs (2)
287-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that a declined Tab reaches normal input handling.
This test only verifies that no away bar appears. It passes if
SimulateKeyswallows the declined Tab.Enable the second input bar, send a short-gap Tab, and assert that the armed bar changes. This verifies the normal Tab path end to end.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs` around lines 287 - 295, Update ATabFromAReaderWhoIsSittingThereIsATab to enable the second input bar, send a short-gap Tab through SimulateKey, and assert that the armed bar changes. Replace the AwayBarIndex-only assertion so the test verifies the declined Tab reaches normal input handling rather than being swallowed.
142-158: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winMake the deep-absence test exercise wrapped display rows.
The bar starts at the beginning of the buffer, and each generated line fits within 120 columns. An implementation that scrolls by buffer index can still pass this test.
Add wrapped content before the departure boundary. Assert the final visible frame through
FrameGrid.Visible, not through a raw ANSI substring search.Based on learnings: use shared
FrameGridhelper for ANSI frame parsing and inspection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs` around lines 142 - 158, Update ADeepAbsenceScrollsThePaneSoTheBarIsOnScreen to add content before the departure boundary that wraps within the pane, ensuring the test exercises display-row scrolling rather than only buffer-index scrolling. After returning from away, inspect the rendered frame via the shared FrameGrid helper and assert against FrameGrid.Visible instead of searching the raw ANSI output.Source: Learnings
src/SharpMUTerm.Tui/SharpMUTermApp.cs (2)
8399-8407: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winClear away-marker state when closing a window.
CloseWindowremoves_lines, but it retains_awayPending,_awayBoundary, and_awayMarks. A spawn window can reopen with the same ID. Its retainedAwayMarkcan then remove a new output row whenRemoveAwayBarruns.Remove all three away-state entries with the window.
Proposed fix
_lines.Remove(id); _freezePoints.Remove(id); + _awayPending.Remove(id);+ _awayBoundary.Remove(id);+ _awayMarks.Remove(id); _workspace.CloseWindow(id);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 8399 - 8407, Update the window cleanup block in CloseWindow to remove the window’s entries from _awayPending, _awayBoundary, and _awayMarks alongside _lines and the other per-window state, ensuring a reopened same-ID window has no stale away-marker state.
2217-2224: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRepaint when trimming removes an away bar.
Line 2222 removes the marker metadata, but the pane control still contains the old bar. Later calls append lines and do not re-feed the control. The stale
AWAYrow remains visible until an unrelated repaint.Repaint this window when trimming removes its marker. Add a regression assertion that
PaneLines(Main)no longer containsAwayBarRenderer.Label.Proposed fix
+ var awayBarTrimmed = false; if (_awayMarks.TryGetValue(windowId, out var mark)) { mark.Index -= excess; if (mark.Index < 0) { _awayMarks.Remove(windowId); + awayBarTrimmed = true; } } if (_panes.TryGetValue(windowId, out var control)) { - control.AppendLine(Compose(buffer[^1]));+ if (awayBarTrimmed)+ {+ RepaintPane(windowId);+ }+ else+ {+ control.AppendLine(Compose(buffer[^1]));+ } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 2217 - 2224, Update the trimming logic around _awayMarks so that when a marker is removed because mark.Index becomes negative, the corresponding window is repainted immediately after _awayMarks.Remove(windowId). Add a regression assertion verifying that PaneLines(Main) no longer contains AwayBarRenderer.Label after trimming removes the marker.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs`:
- Around line 2551-2557: Update RevealAwayBar’s negative-delta scrolling path to
call SyncScrollbackState(windowId) after panel.ScrollVerticalBy(delta), using
the hidden bar’s window identifier rather than ActiveWindowId(). Add a
split-pane test that reveals a bar in an inactive window, then appends output
there and verifies its unread badge/state is preserved.
---
Outside diff comments:
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs`:
- Around line 8399-8407: Update the window cleanup block in CloseWindow to
remove the window’s entries from _awayPending, _awayBoundary, and _awayMarks
alongside _lines and the other per-window state, ensuring a reopened same-ID
window has no stale away-marker state.
- Around line 2217-2224: Update the trimming logic around _awayMarks so that
when a marker is removed because mark.Index becomes negative, the corresponding
window is repainted immediately after _awayMarks.Remove(windowId). Add a
regression assertion verifying that PaneLines(Main) no longer contains
AwayBarRenderer.Label after trimming removes the marker.
In `@tests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs`:
- Around line 287-295: Update ATabFromAReaderWhoIsSittingThereIsATab to enable
the second input bar, send a short-gap Tab through SimulateKey, and assert that
the armed bar changes. Replace the AwayBarIndex-only assertion so the test
verifies the declined Tab reaches normal input handling rather than being
swallowed.
- Around line 142-158: Update ADeepAbsenceScrollsThePaneSoTheBarIsOnScreen to
add content before the departure boundary that wraps within the pane, ensuring
the test exercises display-row scrolling rather than only buffer-index
scrolling. After returning from away, inspect the rendered frame via the shared
FrameGrid helper and assert against FrameGrid.Visible instead of searching the
raw ANSI output.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8878931f-ff1f-4ab0-9c5f-5da019546e13
📒 Files selected for processing (4)
CLAUDE.mddocs/superpowers/specs/2026-08-10-away-divider-design.mdsrc/SharpMUTerm.Tui/SharpMUTermApp.cstests/SharpMUTerm.Tui.Tests/AwayDividerTests.cs
| if (delta < 0) | ||
| { | ||
| // Only ever upwards. A shallow absence leaves the bar and everything under it on screen | ||
| // already, and scrolling then would take a pane off its live tail to reveal something the | ||
| // reader can see — turning a glance into a gesture they have to undo. | ||
| panel.ScrollVerticalBy(delta); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Synchronize the window that RevealAwayBar scrolls.
ScrollVerticalBy raises OnPaneScrolled, but OnPaneScrolled synchronizes ActiveWindowId(). If a hidden bar belongs to an inactive window, that window remains marked as caught up after the reveal. Later output can then miss its unread badge.
Call SyncScrollbackState(windowId) after scrolling. Add a split-pane test that reveals a bar in an inactive window and then appends output to it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/SharpMUTerm.Tui/SharpMUTermApp.cs` around lines 2551 - 2557, Update
RevealAwayBar’s negative-delta scrolling path to call
SyncScrollbackState(windowId) after panel.ScrollVerticalBy(delta), using the
hidden bar’s window identifier rather than ActiveWindowId(). Add a split-pane
test that reveals a bar in an inactive window, then appends output there and
verifies its unread badge/state is preserved.
Tabbing away from the terminal and coming back meant guessing which lines had already been read. Every pane bottom-anchors through an absence, so what you land on is the newest output with no boundary in it — and the unread badges are silent about exactly this case:
Workspace.NoteActivityonly counts a line while a window is notIsCaughtUp, and the window you were looking at when you left stays visible and at its tail the whole time you are gone.This adds a third boundary bar, alongside
FreezeBarRendererandRestoreBarRendererand earning its row the same way — mark the boundary, never restyle the content.The signal, and why both halves are workarounds
Real terminal focus reporting (
CSI ?1004h), which SharpConsoleUI neither asks for nor decodes. Verified against 2.5.18, the newest published (we are pinned at 2.5.14): the assembly's UTF-16 string heap carries[?2004h/[?2004lfor bracketed paste and no?1004in any version;IConsoleDriverhas no focus event;UnixStdinReaderdispatches only key, paste and mouse.So both halves are contained in
TerminalFocusWatcher, one file, so it is one file to delete if a release makes it unnecessary.IConsoleDriver.WriteClipboardOsc52is named for its first customer and is really a verbatim raw write under the renderer's own_consoleLock— the only public write serialised against frame painting, which matters because aConsole.Out.Writeof our own could land mid-frame. Funnelled through oneEmitTerminalMode, so a version that starts validating that payload is one line to fix.AnsiInputParser.DispatchCsireads a trailingIas Tab, which is right forESC [ 1;5 I(Ctrl+Tab in xterm) and wrong for the bare form. Tab is claimed throughRegisterGlobalShortcut's declining overload and deliberately not throughMacroKeys.AppShortcuts— it declines nearly every Tab it sees, and listing it would tell F4's readers a key was gone that is not. A real Tab still reachesInputBarControl's sibling cycle and the settings screens.Focus-out is not recoverable (
ESC [ Ois dropped as anUnknownSequenceEvent), so a departure cannot be timestamped; the boundary anchors to the last input event, which is seconds off. Unix only — the Windows branch is aConsole.ReadKeyloop with its own reassembly — and inert headless, because a harness pressing Tab must get a Tab.The two traps
Telling that Tab from a real one is a question about time, and the comparison must be against the input before it. The disguised focus-in is itself a
KeyPressed, raised beforeInputCoordinatorreaches the global shortcuts, so measuring from the latest timestamp finds a gap of zero on every return and the feature never fires once — a bug indistinguishable from the terminal not supporting?1004.The same trap bit the boundary, one field over. That keypress had already moved
_awayPendingto the end of a buffer full of lines the reader never saw, so_awayBoundarykeeps the value from the input before it.SimulateReturnFromAwaynotes an input first for the same reason: a seam that skipped it would read a boundary the shipping path never reads.Consumption is not
IsCaughtUpThe obvious rule does not survive contact. A bottom-anchored pane is already "visible and not scrolled back" the instant you return, however many hundred lines are above the fold, so clearing on it clears the bar before a word is read. Three conjuncts instead: the bar has been inside the viewport, the pane is at its live tail, and one input has landed since it was drawn — the last of which stops a shallow absence clearing in the frame it appears in.
Also fixed
SimulateKeydiscarded a global shortcut's result and swallowed the key either way. Invisible while every claim returnedtrue; wrong the moment one declined — a harness that ate declined Tabs would have every command-bar cycle test passing against a client that no longer cycles.Verification
dotnet build SharpMUTerm.slnxwarning-free, all five suites green (Core 843, Graphics 83, Scripting 42, Web 37, Tui 1489). New:AwayBarRendererTests,TerminalFocusWatcherTests,AwayDividerTests— including one test that crosses the whole seam through the real global-shortcut registration rather than the simulation seam. Two snapshot views,awayandaway-scrollback, both rendered and looked at.Upstream, worth filing regardless
CSI Iis focus-in, not Tab — a bug on its own terms.FocusInputEvent, a case for it inUnixStdinReader's dispatch, aFocusChangedonIConsoleDriver, and?1004h/?1004lpaired where?2004already is. ~60 lines on the Unix path.WriteRawthat means what it says.If those land, this feature swaps its signal and nothing else moves.
🤖 Generated with Claude Code
https://claude.ai/code/session_0195EnLstmb6tAgpbUHjHBEX
Summary by CodeRabbit