M5: MXP + Pueblo parsers, emoji, clickable links, packaging - #2
Merged
Conversation
Advance MuGlyph into milestone M5 (full parity & polish) with tested, UI-agnostic protocol work plus TUI integration and packaging. Protocols & text (Core) - MxpParser and PuebloParser: incremental, line-oriented parsers that turn markup into styled spans — formatting, colours (WebColors), entities, line breaks, and clickable SEND/A links/commands. Unified behind ILineParser alongside AnsiParser. - SpanInteraction: an optional click behaviour (send-command / hyperlink) on StyledSpan, produced by the parsers and realised by the UI. - EmojiSubstitutor: opt-in emoticon (:) -> emoji) and :shortcode: substitution. - WorldDefinition gains ContentFormat (Ansi/Mxp/Pueblo) and Emoji settings; WorldSession selects the parser and applies emoji per world. TUI (M5 integration) - OutputView tracks per-cell interactions and activates command/link spans on click; MuGlyphApp sends commands / opens links, shows a GMCP-driven stat line, and captures spawn-routed output. Deferred review fixes - TelnetSession: clean up on connect failure and clear the interpreter on disconnect so reconnect works; honour cancellation on send. - WorldSession: dispose a prior session before reconnecting. - KittyGraphicsProtocol: reject placeholder image ids above 24 bits. - ScriptException: also parse the "[source]:LINE:" runtime-error format. Packaging - Self-contained single-file publish profiles (linux-x64, win-x64) and a tagged release workflow; docs/PACKAGING.md. Publish knobs are profile-scoped so ordinary build/run/CI never require a RID. 299 tests pass (Core 200, Graphics 57, Scripting 42). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
|
Important Review skippedToo many files! This PR contains 187 files, which is 37 over the limit of 150. To get a review, narrow the scope: Upgrade to Pro+ to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (201)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds MXP/Pueblo parsing, versioned character-based configuration, workspace and command models, a SharpConsoleUI TUI, an AngleSharp web view, single-file release automation, snapshot tooling, and expanded Core, Web, Graphics, and TUI tests. ChangesMuGlyph platform expansion
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
A text-mode web view: read HTML pages (and follow MXP/Pueblo/HTML links) inside the client, reusing the styled-line + interaction model. MuClient.Web (new, UI-agnostic, tested) - HtmlStyledRenderer: AngleSharp-backed HTML -> StyledLine[]. Block elements become breaks; inline elements map to TextStyle; <a href> and <img> become clickable SpanInteraction spans; headings, lists, <pre>, colours, entities, and word-wrapping to a target width. script/style/head stripped. - WebPageFetcher: http(s)-only fetch with a byte cap and timeout, rendering HTML (or plain text for non-HTML) into a WebPage. - 14 TUnit tests. TUI - WebView pane renders a WebPage's lines with scroll and clickable in-pane navigation (Navigate/Closed events). MuGlyphApp opens it via a `/web <url>` command or by following an output-pane link; Esc closes it. CI runs the new Web test project. 313 tests pass across the solution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/MuClient.Core/Telnet/TelnetSession.cs (1)
244-269: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
_interpreteris nulled before it's ever disposed — interpreter leak on every disconnect/reconnect.
DisposeAsync(lines 281-293) callsDisconnectAsync()first, then only disposes_interpreterif (_interpreter is not null). SinceDisconnectAsyncnow sets_interpreter = nullat Line 267 before returning, that null-check inDisposeAsyncalways fails, so_interpreter.DisposeAsync()is never invoked. BecauseWorldSession.ConnectAsyncalso disposes the previous telnet session on every reconnect, this leaks oneTelnetInterpreterinstance per reconnect, not just at final teardown.🔧 Proposed fix: dispose the interpreter inside `DisconnectAsync` before clearing the field
- // Clear the interpreter so a subsequent ConnectAsync can reconnect and the "not - // connected" guards on the send methods observe the disconnected state. - _interpreter = null; - RaiseDisconnected(null); + // Clear the interpreter so a subsequent ConnectAsync can reconnect and the "not + // connected" guards on the send methods observe the disconnected state. Dispose it + // here (rather than relying on DisposeAsync, which calls DisconnectAsync first and + // would otherwise see it already null). + var interpreter = _interpreter; + _interpreter = null; + if (interpreter is not null) + { + await interpreter.DisposeAsync().ConfigureAwait(false); + } + + RaiseDisconnected(null);🤖 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/MuClient.Core/Telnet/TelnetSession.cs` around lines 244 - 269, Update DisconnectAsync to dispose the current _interpreter asynchronously before setting it to null, preserving the existing disconnected-state reset afterward. Ensure the disposal occurs only when _interpreter is non-null and avoid relying on DisposeAsync to perform cleanup after DisconnectAsync clears the field.
🧹 Nitpick comments (7)
tests/MuClient.Web.Tests/HtmlStyledRendererTests.cs (1)
100-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the image link interaction as well as its label.
This test passes if
srcstops producing an interactive span. AssertIsInteractive, hyperlink kind, and target to protect the advertised behavior.Proposed fix
var span = lines.SelectMany(l => l.Spans).First(s => s.Text.Contains("image")); await Assert.That(span.Text).Contains("a cat"); + await Assert.That(span.IsInteractive).IsTrue(); + await Assert.That(span.Interaction!.Kind).IsEqualTo(InteractionKind.Hyperlink); + await Assert.That(span.Interaction!.Target).IsEqualTo("pic.png");🤖 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/MuClient.Web.Tests/HtmlStyledRendererTests.cs` around lines 100 - 105, Update Image_BecomesLabelledLink to assert the selected span is interactive, has the hyperlink kind, and targets “pic.png”, while retaining the existing assertion that its text contains “a cat”.src/MuClient.Tui/GmcpStats.cs (1)
19-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider unit tests for
GmcpStats.
Update/Summarizeare pure, easily-testable logic (JSON merge + ordering) with no UI dependency, yet no tests are included in this cohort. Coverage here would also have caught the boolean-formatting issue above.Also applies to: 64-89
🤖 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/MuClient.Tui/GmcpStats.cs` around lines 19 - 59, Add unit tests for GmcpStats.Update and Summarize, covering JSON property merging, changed versus unchanged updates, ignored object and array values, invalid or non-object JSON, package filtering, and deterministic summary ordering. Include boolean formatting expectations so the tests catch the referenced formatting issue, while keeping the tests independent of UI components.src/MuClient.Tui/Views/OutputView.cs (1)
20-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated click/interaction-mapping logic between
OutputViewandWebView. Both views independently maintain a(Row, Col) → SpanInteractiondictionary rebuilt every draw, and near-identicalOnMouseEventsingle-click hit-testing — one root cause: no shared base for interactive-span rendering in this TUI.
src/MuClient.Tui/Views/OutputView.cs#L20-L93: extract the_cellInteractionsmap,OnMouseEventhit-test, and per-cell registration-during-draw into a shared base/helper (e.g. anInteractiveTextViewbase or a smallCellInteractionMaputility) that both views compose.src/MuClient.Tui/Views/WebView.cs#L20-L96: haveWebViewreuse the same shared helper instead of re-implementing_cellInteractions/OnMouseEvent, narrowing toInteractionKind.Hyperlinkvia a filter passed to the shared dispatcher.🤖 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/MuClient.Tui/Views/OutputView.cs` around lines 20 - 93, The interaction mapping and click dispatch are duplicated across src/MuClient.Tui/Views/OutputView.cs#L20-L93 and src/MuClient.Tui/Views/WebView.cs#L20-L96. Extract the _cellInteractions storage, per-cell registration, and OnMouseEvent hit-testing into a shared InteractiveTextView base or CellInteractionMap helper; update OutputView to use it for command and hyperlink interactions, and update WebView to reuse it with a filter that accepts only InteractionKind.Hyperlink.tests/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cs (1)
64-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the positive equality case too.
IsNotEqualTopasses even ifSpanInteractiononly has reference equality, so this does not actually verify thatInteractionparticipates inStyledSpanequality/hashing correctly. Add two separately-constructed spans with identical interactions and assert they are equal (and share a hash code).🤖 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/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cs` around lines 64 - 72, Extend StyledSpan_CarriesInteraction_AndAffectsEquality with separately constructed StyledSpan instances using identical SpanInteraction.Command values. Assert these spans are equal and have matching hash codes, while preserving the existing plain-versus-linked inequality assertions to verify interaction value equality and hashing.tests/MuClient.Core.Tests/Protocols/PuebloParserTests.cs (1)
139-147: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing coverage for an unclosed anchor crossing a line boundary.
There is no Pueblo equivalent of
MxpParserTests.Send_BareWithoutClose_ClosesAtEndOfLine, which is why the leak flagged onsrc/MuClient.Core/Protocols/PuebloParser.csLine 398 goes unnoticed. Add a test feeding"<A XCH_CMD=\"look\">here\nnext\n"and assert the second line's span is not interactive.As per coding guidelines: "Maintain unit-test coverage for Core functionality."
🤖 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/MuClient.Core.Tests/Protocols/PuebloParserTests.cs` around lines 139 - 147, Add a Pueblo parser test alongside Anchor_XchCmd_RevertsAfterClose that parses the unclosed anchor input "<A XCH_CMD=\"look\">here\nnext\n" and verifies the span for the second line is not interactive, covering closure at the line boundary.Source: Coding guidelines
tests/MuClient.Core.Tests/Protocols/MxpParserTests.cs (1)
232-240: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExtend this case to cover an interaction with nested formatting crossing the newline.
<SEND HREF="go">walk\nis the easy path; the leak surfaces when a formatting tag is opened inside the send and closed on the next line (see the comment onsrc/MuClient.Core/Protocols/MxpParser.csLine 538). A test likeFeed("<SEND HREF=\"go\"><B>walk\nmore</B> tail\n")asserting line 2's trailing span is non-interactive would lock the fix in.As per coding guidelines: "Maintain unit-test coverage for Core functionality, including ANSI/SGR parsing, telnet round trips, and trigger, alias, and macro engines."
🤖 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/MuClient.Core.Tests/Protocols/MxpParserTests.cs` around lines 232 - 240, Extend Send_BareWithoutClose_ClosesAtEndOfLine to feed a nested formatting tag opened inside the SEND and closed on the following line, such as bold formatting spanning the newline. Assert the second line’s trailing text span is non-interactive, preserving coverage for the interaction-boundary behavior described in MxpParser.Source: Coding guidelines
src/MuClient.Core/Protocols/MxpParser.cs (1)
34-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant namespace qualification.
using MuClient.Core.Text;is already present at Line 3, soMuClient.Core.Text.ILineParsercan just beILineParser(same inPuebloParser).🤖 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/MuClient.Core/Protocols/MxpParser.cs` at line 34, Remove the redundant MuClient.Core.Text namespace qualification from the MxpParser declaration and use the imported ILineParser type directly. Apply the same simplification to the PuebloParser declaration while preserving both classes’ existing interface implementations.
🤖 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 @.github/workflows/release.yml:
- Around line 60-64: Move the “Attach to release” step out of the matrix job and
into a separate downstream release job that depends on both matrix publish jobs
completing. Download or collect both publish artifacts there, then invoke
softprops/action-gh-release@v2 once to upload all assets serially for the tag.
In `@CLAUDE.md`:
- Around line 30-31: Correct the project-count statement in the “M1 delivered,
plus substantial M2–M4 work” section of CLAUDE.md to state that MuGlyph.slnx
builds nine projects: five source projects and four test projects.
In `@docs/PACKAGING.md`:
- Around line 23-28: Update the direct osx-arm64 dotnet publish example in
PACKAGING.md to include the IncludeNativeLibrariesForSelfExtract=true MSBuild
property alongside PublishSingleFile=true, matching the profile-based
single-file configuration.
In `@docs/PLAN.md`:
- Around line 117-125: The in-TUI web view is delivered, but its roadmap and
README statuses are inconsistent. In docs/PLAN.md lines 117-125, remove
WebViewPane from “Still open” or limit the entry to remaining Chromium/image
enhancements; in README.md lines 72-75, update “What works today” to describe
the delivered web view consistently with the roadmap.
In `@src/MuClient.Core/Protocols/MxpParser.cs`:
- Around line 538-557: Update CloseInteractionsAtBoundary and the
formatting-frame cleanup so frames remaining after a line boundary cannot
restore a stale SavedInteraction; clear SavedInteraction (and reset mutable
SpanStart to 0 or otherwise rebase it) for affected frames while preserving
deferred-interaction finalization. Ensure a later CloseTag cannot reinstate the
interaction after _interaction has been cleared.
In `@src/MuClient.Core/Protocols/PuebloParser.cs`:
- Around line 94-105: Update PuebloParser.Flush to call
CloseInteractionsAtBoundary before checking _lineSpans.Count, matching the
boundary handling in MxpParser and ensuring unclosed anchor interactions do not
persist into subsequent flushed lines.
In `@src/MuClient.Core/Session/WorldSession.cs`:
- Around line 177-202: Update ApplyEmoji to evaluate emoji substitution using
the complete StyledLine text, preserving adjacent-character boundary context
across span seams. Remap the substituted text back onto the original StyledSpan
ranges while retaining each span’s Style and Interaction, and return the
original line when no substitution occurs.
In `@src/MuClient.Scripting/ScriptException.cs`:
- Around line 35-56: The TryExtractLine method must anchor line extraction to
MoonSharp script-reference headers instead of scanning arbitrary punctuation in
the message. Update syntax parsing to locate the “:(” marker and runtime parsing
to locate the `"chunk":LINE:` header before parsing the numeric line, then add
regression coverage for messages containing brackets and parentheses while
preserving existing valid syntax and runtime extraction.
In `@src/MuClient.Tui/GmcpStats.cs`:
- Line 50: Update the stat value conversion around property.Value.ToString() in
GmcpStats so JsonValueKind.True and JsonValueKind.False render as lowercase
“true” and “false”; preserve the existing conversion behavior for numeric,
string, and other JSON value kinds.
In `@src/MuClient.Tui/MuGlyphApp.cs`:
- Around line 175-197: Handle exceptions from the fire-and-forget LoadWebAsync
call initiated by OpenWeb, including failures from FetchAsync or disposal races,
and surface a clear error through the existing active/system message mechanism
instead of allowing the task exception to go unobserved. Preserve the current
successful web-view display behavior.
In `@src/MuClient.Tui/Views/WebView.cs`:
- Around line 76-81: Update WebView.Scroll so the maximum scroll offset accounts
for the viewport height, limiting _scroll to the last offset that displays a
full screen of content rather than _lines.Count - 1. Preserve the zero lower
bound and existing SetNeedsDraw behavior, using the view’s available height
symbol.
In `@src/MuClient.Web/HtmlStyledRenderer.cs`:
- Around line 89-94: Update the "hr" handling in HtmlStyledRenderer so the
horizontal rule uses the renderer’s configured/requested width instead of the
hardcoded 40-column length. Preserve the existing styling, preformatted output,
line breaks, and blank lines while sizing the generated rule to that width.
In `@src/MuClient.Web/LineWriter.cs`:
- Around line 139-160: The span coalescing loop should avoid repeated string
concatenation that causes quadratic allocations for large preformatted content.
Replace the accumulated runText handling in the visible coalescing method with a
StringBuilder, append matching segment text, flush its value when styles or
links change and at the end, and add a regression test covering many same-style
preformatted spans.
In `@src/MuClient.Web/WebPageFetcher.cs`:
- Around line 54-68: Update the fetch flow to derive finalUri from
response.RequestMessage?.RequestUri ?? uri after redirects, then use finalUri
for HtmlStyledRenderer and both WebPage constructors so page identity and
relative links resolve from the final location. Add a regression test covering
an automatic redirect and verifying the final URI is used.
---
Outside diff comments:
In `@src/MuClient.Core/Telnet/TelnetSession.cs`:
- Around line 244-269: Update DisconnectAsync to dispose the current
_interpreter asynchronously before setting it to null, preserving the existing
disconnected-state reset afterward. Ensure the disposal occurs only when
_interpreter is non-null and avoid relying on DisposeAsync to perform cleanup
after DisconnectAsync clears the field.
---
Nitpick comments:
In `@src/MuClient.Core/Protocols/MxpParser.cs`:
- Line 34: Remove the redundant MuClient.Core.Text namespace qualification from
the MxpParser declaration and use the imported ILineParser type directly. Apply
the same simplification to the PuebloParser declaration while preserving both
classes’ existing interface implementations.
In `@src/MuClient.Tui/GmcpStats.cs`:
- Around line 19-59: Add unit tests for GmcpStats.Update and Summarize, covering
JSON property merging, changed versus unchanged updates, ignored object and
array values, invalid or non-object JSON, package filtering, and deterministic
summary ordering. Include boolean formatting expectations so the tests catch the
referenced formatting issue, while keeping the tests independent of UI
components.
In `@src/MuClient.Tui/Views/OutputView.cs`:
- Around line 20-93: The interaction mapping and click dispatch are duplicated
across src/MuClient.Tui/Views/OutputView.cs#L20-L93 and
src/MuClient.Tui/Views/WebView.cs#L20-L96. Extract the _cellInteractions
storage, per-cell registration, and OnMouseEvent hit-testing into a shared
InteractiveTextView base or CellInteractionMap helper; update OutputView to use
it for command and hyperlink interactions, and update WebView to reuse it with a
filter that accepts only InteractionKind.Hyperlink.
In `@tests/MuClient.Core.Tests/Protocols/MxpParserTests.cs`:
- Around line 232-240: Extend Send_BareWithoutClose_ClosesAtEndOfLine to feed a
nested formatting tag opened inside the SEND and closed on the following line,
such as bold formatting spanning the newline. Assert the second line’s trailing
text span is non-interactive, preserving coverage for the interaction-boundary
behavior described in MxpParser.
In `@tests/MuClient.Core.Tests/Protocols/PuebloParserTests.cs`:
- Around line 139-147: Add a Pueblo parser test alongside
Anchor_XchCmd_RevertsAfterClose that parses the unclosed anchor input "<A
XCH_CMD=\"look\">here\nnext\n" and verifies the span for the second line is not
interactive, covering closure at the line boundary.
In `@tests/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cs`:
- Around line 64-72: Extend StyledSpan_CarriesInteraction_AndAffectsEquality
with separately constructed StyledSpan instances using identical
SpanInteraction.Command values. Assert these spans are equal and have matching
hash codes, while preserving the existing plain-versus-linked inequality
assertions to verify interaction value equality and hashing.
In `@tests/MuClient.Web.Tests/HtmlStyledRendererTests.cs`:
- Around line 100-105: Update Image_BecomesLabelledLink to assert the selected
span is interactive, has the hyperlink kind, and targets “pic.png”, while
retaining the existing assertion that its text contains “a cat”.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0d17429-5fa1-4f26-a2d0-06d120e03ac2
📒 Files selected for processing (41)
.github/workflows/ci.yml.github/workflows/release.ymlCLAUDE.mdDirectory.Packages.propsMuGlyph.slnxREADME.mddocs/PACKAGING.mddocs/PLAN.mdsrc/MuClient.Core/Configuration/WorldDefinition.cssrc/MuClient.Core/Protocols/MxpParser.cssrc/MuClient.Core/Protocols/PuebloParser.cssrc/MuClient.Core/Session/WorldSession.cssrc/MuClient.Core/Telnet/TelnetSession.cssrc/MuClient.Core/Text/AnsiParser.cssrc/MuClient.Core/Text/EmojiSubstitutor.cssrc/MuClient.Core/Text/ILineParser.cssrc/MuClient.Core/Text/SpanInteraction.cssrc/MuClient.Core/Text/StyledSpan.cssrc/MuClient.Core/Text/WebColors.cssrc/MuClient.Graphics/KittyGraphicsProtocol.cssrc/MuClient.Scripting/ScriptException.cssrc/MuClient.Tui/GmcpStats.cssrc/MuClient.Tui/MuClient.Tui.csprojsrc/MuClient.Tui/MuGlyphApp.cssrc/MuClient.Tui/Properties/PublishProfiles/linux-x64.pubxmlsrc/MuClient.Tui/Properties/PublishProfiles/win-x64.pubxmlsrc/MuClient.Tui/Views/OutputView.cssrc/MuClient.Tui/Views/WebView.cssrc/MuClient.Web/HtmlStyledRenderer.cssrc/MuClient.Web/LineWriter.cssrc/MuClient.Web/MuClient.Web.csprojsrc/MuClient.Web/WebPage.cssrc/MuClient.Web/WebPageFetcher.cstests/MuClient.Core.Tests/Protocols/MxpParserTests.cstests/MuClient.Core.Tests/Protocols/PuebloParserTests.cstests/MuClient.Core.Tests/Session/WorldSessionContentTests.cstests/MuClient.Core.Tests/Text/EmojiSubstitutorTests.cstests/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cstests/MuClient.Graphics.Tests/KittyImageIdValidationTests.cstests/MuClient.Web.Tests/HtmlStyledRendererTests.cstests/MuClient.Web.Tests/MuClient.Web.Tests.csproj
Resolve the actionable CodeRabbit findings on PR #2. Correctness - TelnetSession: dispose the interpreter inside DisconnectAsync before clearing it, so it no longer leaks once per disconnect/reconnect. - MxpParser/PuebloParser: close interaction frames at line/prompt boundaries so an unclosed or nested <SEND>/<A> can't make later lines clickable; Pueblo now mirrors MXP (CompleteLine + Flush). Regression tests added. - WorldSession/EmojiSubstitutor: substitute emoji across the whole line (EmojiSubstitutor.ApplyToLine) so word boundaries are judged correctly across span seams, preserving per-span style/interaction. Tests added. - ScriptException: anchor line extraction to the ":(" / "]:" header markers so a bracket/paren in the message text can't mislead it. - GmcpStats: render JSON booleans as lowercase true/false. - WebPageFetcher: resolve page identity and relative links against the final (post-redirect) URI. - LineWriter: StringBuilder coalescing to avoid O(n^2) on large same-style runs. - HtmlStyledRenderer: size <hr> to the requested width. - MuGlyphApp: observe and surface exceptions from the fire-and-forget web fetch. - WebView: clamp scroll to the last full screen. Delivery / docs - release.yml: serialize release creation in a downstream job so matrix jobs don't race softprops/action-gh-release on the same tag. - Doc fixes: nine projects (CLAUDE.md), web-view marked delivered (PLAN/README), osx-arm64 single-file native-libs note (PACKAGING.md), drop redundant namespace qualification on the parsers. 323 tests pass (Core 209, Graphics 57, Scripting 42, Web 15). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
HarryCordewener
force-pushed
the
claude/muglyph-implementation-5f51l3
branch
from
July 28, 2026 01:13
6652d2d to
ebe8f89
Compare
Reshape the config schema around the design's connection model: - WorldDefinition is now a *server* holding CharacterDefinitions; a character is the connection unit and carries login (connect string, auto-login, on-connect/disconnect) and per-character logging. - Automation moves off the world into world-independent TriggerSets on AppConfiguration; characters opt in by name. A session composes its trigger/alias/macro engines from the union of the character's sets. - WorldSession gains Character, SessionKey (world.character), composed ScriptFiles, and character auto-login on connect. SessionManager gets a character+sets Open overload and SessionKey lookup. - Add ConfigurationMigrator (v1→v2): lifts each world's legacy triggers/aliases/macros/logging into a generated trigger set + default character, run automatically on deserialize. - Drop the BeipMU importer (not needed). - WorldDefinition gains an Accent colour for per-world window traceability. Tests updated for the new shape; 325 passing across the solution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Add MuClient.Core.Workspace — a UI-agnostic model of the design's tmux-style pane tree, so the layout logic is fully unit-testable before any Terminal.Gui hosting: - LayoutNode tree: PaneNode leaves (a tab strip of window ids + active tab + frozen flag) and SplitNode interior nodes (row/col, fractional sizes summing to 1). - WorkspaceLayout owns the tree plus focus/zoom state and all mutation: split-focused (moving non-active tabs into a new sibling), close, cycle focus, zoom, freeze, add/remove/move windows, split-with-window at an edge, and reorder active tab. Every op maintains the invariants — no empty panes, no single-child splits, always ≥1 pane, valid focus/zoom — via a prune-and-fix pass that collapses the tree. 17 workspace tests; 342 passing across the solution. Docs updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Add the Workspace aggregate tying the pane tree to per-window state, so the design's tab strips / spawn routing / rail have a pure, tested core ahead of Terminal.Gui hosting: - WorkspaceWindow: per-window state (id, title, WindowKind, owning world.character, unread count, unsent-input marker) independent of pane placement. - Workspace: keeps the layout tree and the window registry consistent — OpenWindow places a tab, CloseWindow removes it (pruning empties), RouteSpawn finds-or-creates a background spawn window per SpawnTarget and accrues unread while it is not the visible tab, ActivateWindow makes it visible and clears unread. - WorkspaceLayout.AddWindow gains an `activate` flag so spawns open in the background. Renamed the namespace MuClient.Core.Workspace → .Workspaces so the Workspace type isn't shadowed by its own namespace leaf. 351 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
77-78: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unsupported macOS packaging claim.
The supplied
.github/workflows/release.ymlpublishes onlylinux-x64andwin-x64, with matching Linux and Windows profiles. Either removemacOShere or add the corresponding workflow matrix and publish profile.Suggested correction
-- **Packaging** — self-contained single-file publishing for Linux/Windows/macOS (see +- **Packaging** — self-contained single-file publishing for Linux/Windows (see🤖 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 `@README.md` around lines 77 - 78, Update the Packaging bullet in README.md to remove the unsupported macOS claim, leaving only Linux and Windows platforms consistent with the release workflow and available publish profiles.
🧹 Nitpick comments (1)
src/MuClient.Core/Workspace/WorkspaceLayout.cs (1)
122-131: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
AddWindowsilently redirects to the focused pane on an unresolvedpaneId.Unlike
MoveWindowToPaneandSplitWithWindow, which returnfalsewhen the target pane id doesn't resolve,AddWindowfalls back toFocusedPanesilently (paneId is null ? FocusedPane : FindPane(paneId) ?? FocusedPane). A caller passing a stale/removed pane id would have the window routed to the wrong pane with no signal. Consider returningbooland failing explicitly when a non-nullpaneIddoesn't resolve, matching sibling APIs.🤖 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/MuClient.Core/Workspace/WorkspaceLayout.cs` around lines 122 - 131, Update AddWindow to return bool and resolve a non-null paneId without falling back to FocusedPane; return false immediately when FindPane cannot resolve the requested pane, otherwise detach and add the window to the resolved or focused pane, then return true. Preserve activation and pruning behavior, and align the result semantics with MoveWindowToPane and SplitWithWindow.
🤖 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/MuClient.Core/Configuration/CharacterDefinition.cs`:
- Around line 12-13: The CharacterDefinition.Password property is serialized
into config.json as plaintext. Update Password and the ConfigurationStore
serialization/save flow to exclude it from default JSON persistence, and
retrieve or store it through an established secure credential-store mechanism
keyed by character; do not leave a plaintext fallback.
In `@src/MuClient.Core/Configuration/ConfigurationMigrator.cs`:
- Around line 66-114: Update MigrateWorld so a newly created trigger set is also
assigned to existing characters when hasAutomation is true, rather than leaving
it orphaned. Preserve the early return for populated characters when no
automation remains; for populated characters with automation, iterate the
characters and add setName to each character’s triggerSets using the existing
JSON structure without overwriting existing references. Add a migration test
covering characters already present alongside legacy automation fields.
In `@src/MuClient.Core/Workspace/WorkspaceLayout.cs`:
- Around line 3-8: The class documentation for WorkspaceLayout should update its
invariant description to allow the terminal state of one lone pane with an empty
Tabs list after the final pane is closed. Keep the existing guarantees for pane
structure and focus, and explicitly indicate that consumers must not assume a
pane always has an active tab.
- Around line 222-244: Update DetachWindow so pane.ActiveIndex is decremented
only when the removed tab index is strictly before the active index; leave it
unchanged when removing the active tab, then retain the existing ClampActive()
call for tail removals. Add or update coverage for removing an active tab from a
non-edge position while preserving non-active removal behavior.
---
Outside diff comments:
In `@README.md`:
- Around line 77-78: Update the Packaging bullet in README.md to remove the
unsupported macOS claim, leaving only Linux and Windows platforms consistent
with the release workflow and available publish profiles.
---
Nitpick comments:
In `@src/MuClient.Core/Workspace/WorkspaceLayout.cs`:
- Around line 122-131: Update AddWindow to return bool and resolve a non-null
paneId without falling back to FocusedPane; return false immediately when
FindPane cannot resolve the requested pane, otherwise detach and add the window
to the resolved or focused pane, then return true. Preserve activation and
pruning behavior, and align the result semantics with MoveWindowToPane and
SplitWithWindow.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 977a5ca3-1d0d-4193-abdb-9d686f4d83a4
📒 Files selected for processing (35)
.github/workflows/release.ymlCLAUDE.mdREADME.mddocs/PACKAGING.mddocs/PLAN.mdsrc/MuClient.Core/Configuration/AppConfiguration.cssrc/MuClient.Core/Configuration/BeipMuImporter.cssrc/MuClient.Core/Configuration/CharacterDefinition.cssrc/MuClient.Core/Configuration/ConfigurationMigrator.cssrc/MuClient.Core/Configuration/ConfigurationStore.cssrc/MuClient.Core/Configuration/TriggerSet.cssrc/MuClient.Core/Configuration/WorldDefinition.cssrc/MuClient.Core/Protocols/MxpParser.cssrc/MuClient.Core/Protocols/PuebloParser.cssrc/MuClient.Core/Session/SessionManager.cssrc/MuClient.Core/Session/WorldSession.cssrc/MuClient.Core/Telnet/TelnetSession.cssrc/MuClient.Core/Text/EmojiSubstitutor.cssrc/MuClient.Core/Workspace/LayoutNode.cssrc/MuClient.Core/Workspace/WorkspaceLayout.cssrc/MuClient.Scripting/ScriptException.cssrc/MuClient.Tui/GmcpStats.cssrc/MuClient.Tui/MuGlyphApp.cssrc/MuClient.Tui/Views/WebView.cssrc/MuClient.Web/HtmlStyledRenderer.cssrc/MuClient.Web/LineWriter.cssrc/MuClient.Web/WebPageFetcher.cstests/MuClient.Core.Tests/Configuration/ConfigurationTests.cstests/MuClient.Core.Tests/Protocols/ParserBoundaryTests.cstests/MuClient.Core.Tests/Session/WorldSessionContentTests.cstests/MuClient.Core.Tests/Session/WorldSessionTests.cstests/MuClient.Core.Tests/Text/EmojiSubstitutorTests.cstests/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cstests/MuClient.Core.Tests/Workspace/WorkspaceLayoutTests.cstests/MuClient.Web.Tests/HtmlStyledRendererTests.cs
💤 Files with no reviewable changes (1)
- src/MuClient.Core/Configuration/BeipMuImporter.cs
🚧 Files skipped from review as they are similar to previous changes (14)
- docs/PACKAGING.md
- src/MuClient.Scripting/ScriptException.cs
- tests/MuClient.Core.Tests/Text/WebColorsAndInteractionTests.cs
- src/MuClient.Core/Telnet/TelnetSession.cs
- src/MuClient.Tui/GmcpStats.cs
- tests/MuClient.Web.Tests/HtmlStyledRendererTests.cs
- src/MuClient.Tui/Views/WebView.cs
- src/MuClient.Tui/MuGlyphApp.cs
- src/MuClient.Web/LineWriter.cs
- src/MuClient.Core/Protocols/MxpParser.cs
- tests/MuClient.Core.Tests/Session/WorldSessionContentTests.cs
- src/MuClient.Web/WebPageFetcher.cs
- src/MuClient.Core/Protocols/PuebloParser.cs
- src/MuClient.Web/HtmlStyledRenderer.cs
- CharacterDefinition.Password is now [JsonIgnore]: never persisted to plaintext config.json (in-memory/session only; secure credential store is a follow-up). - ConfigurationMigrator: when a v1 world already has characters *and* legacy automation, wire the lifted trigger set into each existing character instead of orphaning it in an unreferenced set. - WorkspaceLayout.DetachWindow: decrement the active index only for a tab strictly before it, so removing the active tab keeps focus on the next tab (consistent regardless of position). - WorkspaceLayout.AddWindow returns bool and fails (no-op) on an unresolved paneId, matching MoveWindowToPane/SplitWithWindow. - WorkspaceLayout doc: note the lone empty-pane terminal state so consumers don't assume ActiveTab is always non-null. - README: drop the unsupported macOS packaging claim (workflow builds linux-x64 + win-x64 only). New tests cover the migration-with-existing-characters case, password non-persistence, and active-tab removal. 357 passing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Two more pure, UI-free primitives the Terminal.Gui pane hosting needs, kept in Core so they're unit-tested before any view code: - LayoutSolver: turns the split tree + a bounds rect into per-pane PaneRects, reserving a 1-cell divider between siblings, honouring each split's fractional sizes (cumulative rounding so no cell drifts), and collapsing to the single zoomed pane when one is zoomed. Because geometry is derived from the bounds on every call, a terminal resize is just a re-solve with the new bounds — the view re-solves per draw. - PaneCommands: the tmux ⌃B keymap (| - z o x < >) → PaneCommand, plus an Apply that drives the WorkspaceLayout, keeping the keymap out of view code. 379 passing across the solution. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Replace the prerelease Terminal.Gui v2 base (whose Application API was [Obsolete] mid-migration and which had no native inline-graphics) with SharpConsoleUI — a stable, net10-targeting compositor framework that natively provides split layouts, tabs, resizable/mouse windows, Spectre-style markup, and the Kitty graphics protocol. The switch is contained to MuClient.Tui because MuClient.Core is UI-agnostic; Core/Scripting/Web/Graphics are untouched. - MuGlyphApp rebuilt on ConsoleWindowSystem + WindowBuilder/Controls: a MarkupControl output pane, a PromptControl command input, a status line, Ctrl+Q quit, background events marshalled via EnqueueOnUIThread, and NAWS re-advertised on window resize. - MarkupFormatter converts the UI-agnostic StyledLine model to Spectre-style markup: truecolor fg/bg via the theme, bold/italic/ underline/etc., and clickable MXP/Pueblo/web spans as [link=…] (custom mux:send:/mux:prompt: schemes dispatched on LinkClicked). Bracket escaping included. - The web view opens as a second window rendering the page as markup. - Removed the Terminal.Gui views (OutputView/CommandInput/WebView/ ColorMapper); swapped the package reference. - New MuClient.Tui.Tests (TUnit) covers MarkupFormatter; wired into CI. 387 passing across the solution (Tui build-verified; visual pass on a real terminal, per project norms). Docs (README/CLAUDE/PLAN) updated to record SharpConsoleUI as the TUI base. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Turn the single output pane into a SharpConsoleUI TabControl of output windows, driven by the tested Core.Workspaces model (a single pane whose tabs are the windows): - Each window (main + trigger-routed spawn windows + the web view) is a MarkupControl tab; the tab's Tag carries the window id. - A trigger SpawnTarget now actually routes its line to a spawn window (created on first use) instead of just announcing it; background tabs accrue an unread badge via Workspace.RouteSpawn/NoteActivity. - Selecting a tab activates that window in the model (clearing unread); TabTitles renders each header as "title (unread) ✎" from window state. - The web view opens as a tab rather than a separate window. TabTitles is a pure helper with unit tests; 391 passing across the solution (Tui build-verified, visual pass on a real terminal). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The single command prompt now remembers a separate draft per window: typing stores the draft for the visible tab and sets its unsent-input marker (Workspace.SetUnsentInput → the ✎ badge); switching tabs restores that window's draft; sending or clearing the line drops the draft and marker. Builds on the already-tested Workspace state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Make the tabbed windows navigable from the keyboard: Ctrl+N (and Ctrl+Tab where the terminal reports it) cycles to the next window tab; Ctrl+W closes the active spawn/web window (the main window is protected) and drops its pane, draft, and Workspace state. Ctrl+Q still quits. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Add a way to capture images of the UI with no terminal or connection — using SharpConsoleUI's HeadlessConsoleDriver: - `muglyph --snapshot [--size WxH] [--out file]` builds the app on the headless driver, loads a demo scene (a room with clickable exits, a Chat spawn window with unread, an input draft), renders one frame, and writes the raw ANSI. Force-exits after capture (the framework keeps worker threads alive). - tools/ansi_frame_to_image.py parses the cursor-addressed truecolor frame into a grid and emits a self-contained SVG or HTML (no deps). - tools/make-screenshots.sh regenerates docs/screenshots/*. - docs/SCREENSHOTS.md documents the pipeline (+ VHS for animated demos). - README embeds the generated screenshot. The frame is deterministic (desktop panels off under headless), so it can double as a CI golden file. Committed a first demo frame + SVG/HTML. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Remove the README screenshot embed and the committed sample frames — the current shell isn't at the design's fidelity yet, so it shouldn't be the project's face. The snapshot tooling (muglyph --snapshot, tools/ansi_frame_to_image.py, make-screenshots.sh, docs/SCREENSHOTS.md) stays so images can be regenerated once the UI matches the design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Add the tested, UI-agnostic models the design's UI needs, so the view layer is correct-by-construction and verifiable: - Commands: CommandItem/CommandGroup, CommandCatalog (generates the ⌃P catalog from live workspace state — switch-character and go-to-window entries, stateful logging/zoom/freeze labels, GO TO/WORLD/TERMINAL/ LAYOUT groups) and CommandMatcher (prefix > substring > subtitle > fuzzy-subsequence ranking, case-insensitive, empty query = all). - Workspaces.RailModel: projects worlds→characters→windows into the connection-rail rows (accent, connected dot, active marker, unread, ✎ unsent, hosting pane; windows expanded only under the active character; "no characters" placeholder). - Text.Meters (HP/EN bars + keepalive sparkline) and Text.StatusFormatter (Corvid@aetherfall › prompt + destination/drafts/char-count gutter). 24 new tests; 285 Core tests, all green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Wire the design's command surface: Ctrl+P toggles a modal search box over the live-generated catalog (CommandCatalog), ranked by CommandMatcher and rendered grouped GO TO / WORLD / TERMINAL / LAYOUT by CommandSurfaceRenderer (pure, unit-tested — Order + Render). ↑↓ walk the flattened results, ⏎ dispatches the selected id, Esc closes. DispatchCommand runs what the current tabbed shell supports (go-to window, zoom/freeze/close/split on the layout model, clear window, reconnect/disconnect) and notes commands not yet wired. CommandSurfaceRenderer has its own tests; solution builds, Core 285 + Tui 15 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Restructure the window toward the design's regions: a header row (☰ glyph·tui + log/graphics/⌃P hints, sticky top), the tabbed pane area (fill), the input prompt (sticky bottom, now the › glyph), and a status bar (sticky bottom): connection dot + character + state, HP/EN meters rendered from GMCP Char.Vitals via Core.Text.Meters, host, and the palette hint. GmcpStats.GetInt feeds the meters. The demo scene seeds sample vitals so snapshots show the bars. Builds; 300 tests green (Core 285 + Tui 15). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Replace the Run()-on-a-worker-thread render (which raced the framework's input+render pump and hung) with a single inline ConsoleWindowSystem .ForceRender() — one render cycle, no loop, no thread, no exit race. The HeadlessConsoleDriver writes the composited frame to the console, which we capture by redirecting Console.Out for that one call. Measured end to end: ctor+scene+render ≈ 240ms, deterministic, exit 0. Fix identified by a background investigation agent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Adds the left connection rail from the design handoff. RailRenderer (pure, tested) maps RailModel rows to markup: a CONNECTIONS header, each world with an accent spine and host line, characters with a connected dot (●/○) and active marker (▸), and — under the active character only — its windows with unread, unsent (✎), and hosting-pane detail. A world with no characters prints "no characters". MuGlyphApp hosts it in a HorizontalGrid (rail column 30 wide + splitter + the pane area flexing to fill), projects config worlds + workspace windows into rail rows via BuildRail(), and refreshes on every activity/connection change. Accents fall back to a per-world palette when a world hasn't set its own. The headless demo scene seeds two demo worlds so snapshots exercise the rail. Verified by a 120x32 --snapshot frame reconstruction: header, world spines, active/inactive characters, window rows, splitter, and status meters all render. 421 tests green (7 new RailRenderer tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Threads the world accent (its own, or a per-world palette fallback) through the chrome so the regions read as one workspace: the header shows the active world with its accent spine and character (`▚ Aetherfall · Corvid`) plus a connected-count summary, and the status bar's `●` picks up the same accent. The rail already carried accents; ActiveWorld()/AccentFor()/AccentHex() centralise the lookup shared by all three. Also fixes a real, non-deterministic snapshot hang: `--snapshot` constructed the window system while stdin was still attached, and the framework's console input setup blocks forever when stdin is an interactive TTY or an open pipe — so the frame never rendered when launched in the foreground (it only "worked" when stdin happened to be /dev/null, e.g. backgrounded or redirected). Detaching stdin (Console.SetIn(TextReader.Null)) in the snapshot branch makes it return EOF immediately, so the frame renders deterministically however it's launched. Proven: 4/4 foreground runs now emit an identical 12757-byte frame where they previously hung. 421 tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Completes the input region from the design. The prompt is now bound to the focused character (`Corvid@Aetherfall ›` rather than a bare `›`), and a thin dim gutter sits above it showing the destination window, any other windows holding unsent drafts, and the live character count (`→ Aardwolf 15`). Both strings come from the already-tested StatusFormatter; UpdateInputChrome() refreshes them on every input/tab/activity change. Verified via a 120x30 snapshot reconstruction: gutter and character prompt render in order above the status bar. 307 Core+Tui tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The workspace split-tree model was fully wired (split-right/down move non-active tabs into a sibling pane) but the view only ever showed one TabControl. Now the pane area renders the tree itself: each leaf pane is a TabControl, a row split becomes a proportional column grid and a column split a row grid, with a draggable splitter between children, and a zoomed pane renders full-area. Tab management is now model-driven end to end. BuildWorkspaceRow() projects the layout (honouring zoom) into a control tree tracked by pane id; RebuildPaneArea() swaps the fresh row into the live window (RemoveContent + InsertControl) on every layout change — split, close, zoom, and first-seen spawn/web windows. NextWindow, CloseActiveWindow, Activate, and tab-change all operate on the focused pane's TabControl, and the split commands from the ⌃P surface now visibly reshape the workspace. Per-pane content controls (scrollback) persist across rebuilds. Proven via snapshot: single pane renders unchanged (no regression), and a forced split renders two panes side by side — left "Aardwolf" showing the room, right "Chat" showing the chat lines, divider between — matching the rail's pane labels. 307 Core+Tui tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Adds Ctrl+O to move focus to the next pane in a split, routing the input line to that pane's active window (draft restored) and focusing its tab strip — splits are navigable from the keyboard, not just the ⌃P surface. Docs: README and PLAN now describe the delivered multi-pane workspace (rail, split panes, command surface, accents, status meters, input gutter) instead of listing it as upcoming, and the "still open" list narrows to freeze-view, settings dialogs, and mouse/drag. Reconciles the web-view status the roadmap and README already agreed on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Adds a CI step that runs `muglyph --snapshot` and asserts the frame contains the workspace markers (CONNECTIONS rail header, a seeded world, a character, the palette hint). Because snapshot mode renders one frame and exits, a return of the stdin-block hang would stall the step (caught by the job timeout), and any UI-render regression drops a token — turning the manual snapshot checks into an automated end-to-end guard. Verified locally against the Release build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
WorkspaceState captures/restores a workspace — its windows (id/title/kind/ session/owner/capture) and the full pane tree with focus and zoom — so the last session can be saved and resumed. Restore rebuilds a live Workspace via new restore constructors on Workspace and WorkspaceLayout (the latter advances its pane counter past restored ids so future splits never collide). Scrollback and transient badges are intentionally excluded. Round-trips through the config's JSON serializer. 4 tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The app now rebuilds its workspace from AppConfiguration.LastSession at construction (ResumeOrNew) — real startup and the demo share the exact 'load config → resume last session' path, so snapshots reflect genuine behaviour rather than a bespoke scene. Corrupt saved state falls back to a fresh workspace. The demo is now a config: DemoScene.Build() assembles worlds, characters, trigger sets, and a saved LastSession (Corvid's main + a Chat spawn in one pane); LoadDemoScene only feeds scrollback into the already-resumed windows. AppConfiguration gains LastSession (round-trips through the store; +1 test). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
On clean exit the live app captures its workspace into config.LastSession and writes it to the default config path, so the next launch resumes the same panes and windows. A save failure is swallowed — it never affects the exit code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Replace the framework's fixed double-line (║/═) grid splitters with thin solid 1-cell dividers in the subtle border colour, so panes and the rail read as a single calm line instead of a busy double rule — and output no longer appears to bleed across the divider. Shorten the wordmark to 'glyph' and render the menu affordance as a filled accent 'button' (menu glyph + wordmark, padded a space each side). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Header is now an Oh-My-Posh-style powerline ribbon — a clickable 'glyph' button (on-brand violet) flowing through the world and character on solid triangle separators () into the menu-bar band (a distinct chrome background). Nerd Font separators, colours resolved through the theme/accents. Powerline is a pure, tested renderer (3 tests). Input region gets its own subtly-elevated background (gutter + prompt) so it's easy to tell apart from the output above and status below, and the gutter's bare char count now reads '15 chars'. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
- Remove the input gutter entirely — the input bar's own background is enough
to set it apart; its character count moves to the status bar ('15 chars', or
the 'back to draft' hint while recalling history).
- Status bar drops the HP/EN meters (kept the keepalive sparkline, host,
encoding) and gains the live char count.
- Header spans the full console: the menu + world/character powerline ribbon
stays left, while the connection/LOG/graphics cluster is right-aligned to the
far edge (re-aligned on resize). The clock is removed.
- Give the input field its own elevated background.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Bump the input field's elevated background to a more distinct blue-grey so the input bar reads apart from the output body at a glance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (10)
tests/MuClient.Core.Tests/Text/MetersAndStatusTests.cs (1)
27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the obsolete
HasLengthassertion.Proposed fix
- await Assert.That(spark).HasLength(4); + await Assert.That(spark).Length().IsEqualTo(4);#!/bin/bash set -eu rg -n 'HasLength\(|Length\(\)\.IsEqualTo' \ tests/MuClient.Core.Tests/Text/MetersAndStatusTests.cs🤖 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/MuClient.Core.Tests/Text/MetersAndStatusTests.cs` at line 27, Replace the obsolete HasLength assertion for spark with the current supported assertion API, preserving the expected length of 4. Update any matching obsolete length assertions in MetersAndStatusTests identified by the provided search.tools/make-screenshots.sh (2)
24-25: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueParses the same frame twice; reuse the grid or note the intent. Minor, but each
renderinvocation re-reads and re-parses$name.ansito emit SVG then HTML. Adding a dual-output mode toansi_frame_to_image.pywould halve the work and keep both outputs provably in sync.🤖 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 `@tools/make-screenshots.sh` around lines 24 - 25, Update the screenshot generation flow around ansi_frame_to_image.py so each $name.ansi frame is parsed only once while producing both the SVG and HTML outputs. Add or use a dual-output capability that writes the two requested destinations from the same parsed grid, preserving the existing output filenames and formats.
16-23: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winHardcoded DLL path breaks if the assembly name or output layout changes.
$ROOT/src/MuClient.Tui/bin/Release/net10.0/muglyph.dllassumes the assembly is namedmuglyphand that noRuntimeIdentifieris set (which would nest the output undernet10.0/<rid>/). The publish profiles in this PR add RID-specific publishing, so this is worth pinning down. Driving the app throughdotnet run --projectavoids the guess entirely and matches how the rest of the repo invokes .NET projects.♻️ Proposed refactor
-echo "Building muglyph…" -dotnet build "$ROOT/src/MuClient.Tui/MuClient.Tui.csproj" -c Release >/dev/null - -DLL="$ROOT/src/MuClient.Tui/bin/Release/net10.0/muglyph.dll" +PROJ="$ROOT/src/MuClient.Tui/MuClient.Tui.csproj" + +echo "Building muglyph…" +dotnet build "$PROJ" -c Release >/dev/null render() { local name="$1" size="$2" echo "Rendering $name ($size)…" - dotnet "$DLL" --snapshot --size "$size" --out "$OUT/$name.ansi" + dotnet run --project "$PROJ" -c Release --no-build -- \ + --snapshot --size "$size" --out "$OUT/$name.ansi"Alternatively resolve the path with
dotnet build "$PROJ" -c Release --getProperty:TargetPath.#!/bin/bash # Confirm assembly name and any RuntimeIdentifier in the TUI project fd -g 'MuClient.Tui.csproj' --exec cat -n {} fd -g '*.pubxml' --exec sh -c 'echo "== {}"; cat {}'Based on learnings: "Use
dotnet buildas the build verification signal; run TUnit projects withdotnet run --project, rather than relying ondotnet test."🤖 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 `@tools/make-screenshots.sh` around lines 16 - 23, Replace the hardcoded DLL path used by render() with project-based execution via dotnet run --project, reusing the MuClient.Tui project path and Release configuration established by the build step. Preserve the existing snapshot arguments, size handling, output naming, and build verification behavior without assuming an assembly name or output directory layout.tools/ansi_frame_to_image.py (1)
96-101: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueWide glyphs advance the column by one, so East-Asian/emoji cells shift the rest of the row.
The parser assumes every printable char is one cell. Nerd Font icons and box drawing are single-width so current snapshots are fine, but any double-width codepoint will misalign the remainder of the row against the terminal's own accounting. Worth a note in the docstring or a
unicodedata.east_asian_widthcheck if snapshots ever include such content.🤖 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 `@tools/ansi_frame_to_image.py` around lines 96 - 101, Update the printable-character parsing branch to account for East Asian and emoji double-width glyphs instead of always incrementing col by one. Use a Unicode width check (such as unicodedata.east_asian_width) and advance the column by the glyph’s terminal width so subsequent cells remain aligned.src/MuClient.Core/Workspace/Workspace.cs (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "Terminal.Gui" references in doc comments — the TUI base was renamed to SharpConsoleUI. This PR consistently renames the UI framework everywhere else (CLAUDE.md, docs/PLAN.md, README.md), but these three doc comments still describe a "Terminal.Gui layer"/"Terminal.Gui key handler" consuming the pure Core model.
src/MuClient.Core/Workspace/Workspace.cs#L3-10: update "the Terminal.Gui layer renders from it and calls its operations" to reference SharpConsoleUI.src/MuClient.Core/Workspace/LayoutSolver.cs#L10-15: update "the Terminal.Gui layer maps the resulting PaneRects onto views" to reference SharpConsoleUI.src/MuClient.Core/Workspace/PaneCommands.cs#L31-36: update "the Terminal.Gui key handler resolves a key here" to reference SharpConsoleUI.As per coding guidelines,
**/MuClient.Core/**/*.cs: "KeepMuClient.CoreUI-agnostic: transport, telnet, parsing, routing, scrollback, engines, and logging must not depend on UI frameworks." — leaving stale, wrong-framework references in Core's own doc comments works against that agnosticism in spirit and could mislead future contributors about which framework actually consumes these models.🤖 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/MuClient.Core/Workspace/Workspace.cs` at line 1, Update the XML documentation comments in Workspace, LayoutSolver, and PaneCommands to replace each stale “Terminal.Gui” reference with “SharpConsoleUI,” preserving the existing descriptions and keeping MuClient.Core implementation UI-agnostic.Source: Coding guidelines
src/MuClient.Tui/MuGlyphApp.cs (1)
1067-1079: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
titleparameter is unused.
PaneContentFornever readstitle; every call site computes one anyway. Drop it, or use it (e.g. for an accessibility/debug label).🤖 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/MuClient.Tui/MuGlyphApp.cs` around lines 1067 - 1079, The title parameter is unused in PaneContentFor. Remove title from the method signature and update every call site to stop computing and passing it, unless the parameter is intentionally needed for an accessibility or debugging label.src/MuClient.Tui/WorldsScreenRenderer.cs (1)
128-135: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valuePadding is applied to the escaped string.
Escape(...).PadRight(13)counts[[/]]as two columns each, so a name containing a bracket misaligns the row. Pad on visible width (you already haveVisibleLength) instead.🤖 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/MuClient.Tui/WorldsScreenRenderer.cs` around lines 128 - 135, Update CharacterRow so the escaped character name is padded based on visible display width rather than raw markup length. Reuse the existing VisibleLength helper when calculating the required padding, preserving the current 13-column layout and escaping behavior.src/MuClient.Tui/MarkupFormatter.cs (1)
131-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
tokens.Count == 0is unreachable.
Hex(fg)is always added just above, so the null branch never fires and every span gets a tag. Either drop the dead check or skip tag emission when the style is fully default (cheaper output).🤖 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/MuClient.Tui/MarkupFormatter.cs` around lines 131 - 141, The tokens.Count == 0 check in the markup formatter is unreachable because Hex(fg) is always added. Update the surrounding formatter method to remove the dead null branch and return the generated tag directly, or explicitly detect a fully default style before adding tokens so default spans can omit tag emission.src/MuClient.Tui/RailRenderer.cs (1)
49-55: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
Label[..1]can split a surrogate pair.For a name starting with a non-BMP rune the collapsed rail emits a lone surrogate.
StringInfo.GetTextElementEnumerator/EnumerateRuneswould give a safe first grapheme.🤖 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/MuClient.Tui/RailRenderer.cs` around lines 49 - 55, Update the initial-character extraction in the RailRenderer character-row branch to avoid slicing Label with Label[..1], which can split surrogate pairs. Use EnumerateRunes or StringInfo.GetTextElementEnumerator to obtain the first complete grapheme/rune while preserving the existing "?" fallback and Escape processing.src/MuClient.Tui/TimersScreenRenderer.cs (1)
38-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLeft column isn't padded, so the
│divider is ragged.
TriggersScreenRendererandWorldsScreenRendererboth pad the left column to a fixed visible width before joining; this one concatenates raw, so the divider zig-zags with row length.🤖 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/MuClient.Tui/TimersScreenRenderer.cs` around lines 38 - 44, Update the row construction in the timer screen renderer to pad each left-column value to the same fixed visible width used by TriggersScreenRenderer and WorldsScreenRenderer before appending the divider. Preserve the existing empty-value handling and right-column placement while ensuring every “│” aligns vertically.
🤖 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 45-49: Update the Tui bullet in CLAUDE.md to remove the stale
“splits ... + rail layer on next” wording and describe split panes and the
connection rail as already rendered by the SharpConsoleUI shell, consistent with
docs/PLAN.md and README.md.
In `@docs/design/README.md`:
- Line 111: Correct the “Connection rail” description in the design README to
label 204 and 46 as pixel widths, not column counts, and align the wording with
the approximate column conversions documented later on the referenced line.
- Around line 15-21: The design documentation contains stale framework and
file-path references. Update docs/design/README.md to identify
Glyph-TUI-v3.dc.html correctly and replace Terminal.Gui v2, Theme, and
ColorMapper guidance with SharpConsoleUI guidance scoped to MuClient.Tui, while
preserving the instruction to rebuild the screens rather than port the HTML
structure.
In `@src/MuClient.Core/Commands/CommandCatalog.cs`:
- Line 3: Convert the namespace declarations to file-scoped form and remove each
corresponding closing brace: update src/MuClient.Core/Commands/CommandCatalog.cs
(3-3), src/MuClient.Core/Text/StatusFormatter.cs (1-1), and
src/MuClient.Tui/TabTitles.cs (3-3), plus
tests/MuClient.Tui.Tests/TimersScreenRendererTests.cs (5-5),
TriggersScreenRendererTests.cs (6-6), WorldsScreenRendererTests.cs (6-6),
AliasesScreenRendererTests.cs (5-5), CaptureLineRendererTests.cs (3-3),
CommandSurfaceRendererTests.cs (4-4), KeypadScreenRendererTests.cs (4-4), and
RailRendererTests.cs (5-5). Preserve all existing members while ensuring every
affected C# file uses a file-scoped namespace.
In `@src/MuClient.Core/Commands/CommandMatcher.cs`:
- Around line 79-111: Update FuzzySpread to evaluate all valid fuzzy subsequence
matches rather than greedily fixing the first matching start, and return the
smallest span between matched characters. Preserve null for queries that cannot
be matched while ensuring tighter runs receive the better score.
In `@src/MuClient.Core/Workspace/WorkspaceState.cs`:
- Around line 103-112: Update BuildNode to explicitly handle the "pane" type
when creating a PaneNode, and reject any other unknown LayoutNodeState.Type
value instead of treating it as a pane. Preserve the existing case-insensitive
"split" handling and use the established exception pattern for invalid
serialized node types.
In `@src/MuClient.Core/Workspaces/RailModel.cs`:
- Around line 66-72: Update the world-row construction flow around
RailRowKind.World to add the documented RailRowKind.Host row using the current
RailWorld.Host and RailWorld.Port values, formatted as the expected host:port
display, before processing characters. Preserve the existing empty-character
handling and continuation behavior.
In `@src/MuClient.Tui/AliasesScreenRenderer.cs`:
- Around line 106-108: Update the case-sensitive label in the alias rendering
logic around alias.CaseSensitive so the checkbox brackets are escaped as literal
markup text, while preserving the existing selected/unselected styling and
labels.
In `@src/MuClient.Tui/DemoScene.cs`:
- Around line 96-100: Update the wtf Alias pattern and substitution in the
DemoScene alias configuration so the $1 reference has a corresponding captured
argument, or remove $1 if wtf is intentionally argumentless; preserve the
existing say alias unchanged.
In `@src/MuClient.Tui/MarkupFormatter.cs`:
- Around line 143-151: Update LinkFor’s InteractionKind.Hyperlink branch to
percent-encode square brackets in interaction.Target before embedding it in the
link markup, while preserving the rest of the URL unchanged. Ensure
OnLinkClicked decodes the same encoding symmetrically when handling hyperlink
targets.
In `@src/MuClient.Tui/MuGlyphApp.cs`:
- Around line 1542-1554: Update CloseActiveWindow to also remove the closed
window’s entries from _lines and _freezePoints, alongside the existing _panes
and _drafts cleanup. Ensure reopening a window with the same
Workspace.SpawnWindowId does not reuse old scrollback or freeze-point state.
- Around line 469-481: Update AppendWindowLine to enforce
_config.ScrollbackLines when adding markup to each window buffer, trimming the
oldest entries whenever the buffer exceeds the configured limit. Keep the pane
append behavior unchanged and ensure BuildFrozenContent receives only the
bounded scrollback history.
- Around line 1441-1465: Update HandleMoveKey so _moveTargetPaneId is assigned
only when _moveLetters contains a pane whose value matches ch; leave the
existing target unchanged for unmatched move-mode letters, while preserving the
current rebuild and status-update behavior for valid matches.
In `@src/MuClient.Tui/OptionsScreenRenderer.cs`:
- Around line 53-57: Escape checkbox brackets in RenderRow, RightColumn, and
BuildEditor so all toggle glyphs emit markup-safe [[x]] and [[ ]] values;
optionally centralize this formatting in a shared Checkbox(bool) helper and use
it across the three affected files: src/MuClient.Tui/OptionsScreenRenderer.cs
lines 53-57, src/MuClient.Tui/TimersScreenRenderer.cs lines 100-101, and
src/MuClient.Tui/TriggersScreenRenderer.cs lines 167-169.
In `@tests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cs`:
- Around line 23-24: Update the assertion for CommandSurfaceRenderer.Order in
the ordered ID test to use IsEquivalentTo with CollectionOrdering.Matching,
preserving the expected sequence and making the comparison order-sensitive.
In `@tools/ansi_frame_to_image.py`:
- Around line 11-12: Update the usage examples in the ansi_frame_to_image.py
module docstring to invoke ansi_frame_to_image.py instead of
ansi_frame_to_svg.py, preserving the existing command arguments and output
behavior.
- Around line 231-241: Update main to separate the --html flag from positional
input and output paths before reading data or assigning out_path. Ensure
stdin+--html and input-path+--html treat --html only as a flag, while preserving
HTML inference from .html output paths. Open input and output files with
deterministic context-managed handling.
- Around line 106-124: Extend apply_sgr to consume 256-colour sequences 38;5;<n>
and 48;5;<n> as complete parameters, preventing their components from being
processed as standalone SGR codes. Handle 39 and 49 by restoring DEFAULT_FG and
DEFAULT_BG, and map basic foreground/background palette codes, including bright
variants, to their corresponding colours while preserving existing truecolor and
attribute behavior.
In `@tools/fonts/README.md`:
- Around line 11-19: Add the complete SIL Open Font License 1.1 text and
copyright notice as tools/fonts/OFL.txt, add the applicable Nerd Fonts MIT
license text in the same directory, and update the Licensing section of the
README to reference both bundled files alongside the existing attribution and
usage details.
---
Nitpick comments:
In `@src/MuClient.Core/Workspace/Workspace.cs`:
- Line 1: Update the XML documentation comments in Workspace, LayoutSolver, and
PaneCommands to replace each stale “Terminal.Gui” reference with
“SharpConsoleUI,” preserving the existing descriptions and keeping MuClient.Core
implementation UI-agnostic.
In `@src/MuClient.Tui/MarkupFormatter.cs`:
- Around line 131-141: The tokens.Count == 0 check in the markup formatter is
unreachable because Hex(fg) is always added. Update the surrounding formatter
method to remove the dead null branch and return the generated tag directly, or
explicitly detect a fully default style before adding tokens so default spans
can omit tag emission.
In `@src/MuClient.Tui/MuGlyphApp.cs`:
- Around line 1067-1079: The title parameter is unused in PaneContentFor. Remove
title from the method signature and update every call site to stop computing and
passing it, unless the parameter is intentionally needed for an accessibility or
debugging label.
In `@src/MuClient.Tui/RailRenderer.cs`:
- Around line 49-55: Update the initial-character extraction in the RailRenderer
character-row branch to avoid slicing Label with Label[..1], which can split
surrogate pairs. Use EnumerateRunes or StringInfo.GetTextElementEnumerator to
obtain the first complete grapheme/rune while preserving the existing "?"
fallback and Escape processing.
In `@src/MuClient.Tui/TimersScreenRenderer.cs`:
- Around line 38-44: Update the row construction in the timer screen renderer to
pad each left-column value to the same fixed visible width used by
TriggersScreenRenderer and WorldsScreenRenderer before appending the divider.
Preserve the existing empty-value handling and right-column placement while
ensuring every “│” aligns vertically.
In `@src/MuClient.Tui/WorldsScreenRenderer.cs`:
- Around line 128-135: Update CharacterRow so the escaped character name is
padded based on visible display width rather than raw markup length. Reuse the
existing VisibleLength helper when calculating the required padding, preserving
the current 13-column layout and escaping behavior.
In `@tests/MuClient.Core.Tests/Text/MetersAndStatusTests.cs`:
- Line 27: Replace the obsolete HasLength assertion for spark with the current
supported assertion API, preserving the expected length of 4. Update any
matching obsolete length assertions in MetersAndStatusTests identified by the
provided search.
In `@tools/ansi_frame_to_image.py`:
- Around line 96-101: Update the printable-character parsing branch to account
for East Asian and emoji double-width glyphs instead of always incrementing col
by one. Use a Unicode width check (such as unicodedata.east_asian_width) and
advance the column by the glyph’s terminal width so subsequent cells remain
aligned.
In `@tools/make-screenshots.sh`:
- Around line 24-25: Update the screenshot generation flow around
ansi_frame_to_image.py so each $name.ansi frame is parsed only once while
producing both the SVG and HTML outputs. Add or use a dual-output capability
that writes the two requested destinations from the same parsed grid, preserving
the existing output filenames and formats.
- Around line 16-23: Replace the hardcoded DLL path used by render() with
project-based execution via dotnet run --project, reusing the MuClient.Tui
project path and Release configuration established by the build step. Preserve
the existing snapshot arguments, size handling, output naming, and build
verification behavior without assuming an assembly name or output directory
layout.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5ad53285-dcb2-4269-a20c-861b35ec0b84
⛔ Files ignored due to path filters (1)
tools/fonts/MuGlyphMonoNerd.woffis excluded by!**/*.woff
📒 Files selected for processing (88)
.github/workflows/ci.yml.gitignoreCLAUDE.mdDirectory.Packages.propsMuGlyph.slnxREADME.mddocs/PLAN.mddocs/SCREENSHOTS.mddocs/design/Glyph-TUI-v3.dc.htmldocs/design/README.mddocs/design/support.jssrc/MuClient.Core/Automation/TimerDefinition.cssrc/MuClient.Core/Automation/TriggerEngine.cssrc/MuClient.Core/Commands/CommandCatalog.cssrc/MuClient.Core/Commands/CommandItem.cssrc/MuClient.Core/Commands/CommandMatcher.cssrc/MuClient.Core/Configuration/AppConfiguration.cssrc/MuClient.Core/Configuration/CharacterDefinition.cssrc/MuClient.Core/Configuration/ConfigurationMigrator.cssrc/MuClient.Core/Configuration/TriggerSet.cssrc/MuClient.Core/Configuration/WorldDefinition.cssrc/MuClient.Core/Input/InputHistory.cssrc/MuClient.Core/Text/Meters.cssrc/MuClient.Core/Text/StatusFormatter.cssrc/MuClient.Core/Text/StyledLine.cssrc/MuClient.Core/Workspace/DropZones.cssrc/MuClient.Core/Workspace/LayoutNode.cssrc/MuClient.Core/Workspace/LayoutSolver.cssrc/MuClient.Core/Workspace/PaneCommands.cssrc/MuClient.Core/Workspace/Workspace.cssrc/MuClient.Core/Workspace/WorkspaceLayout.cssrc/MuClient.Core/Workspace/WorkspaceState.cssrc/MuClient.Core/Workspace/WorkspaceWindow.cssrc/MuClient.Core/Workspaces/RailModel.cssrc/MuClient.Tui/AliasesScreenRenderer.cssrc/MuClient.Tui/CaptureLineRenderer.cssrc/MuClient.Tui/ColorMapper.cssrc/MuClient.Tui/CommandPalette.cssrc/MuClient.Tui/CommandSurfaceRenderer.cssrc/MuClient.Tui/DemoScene.cssrc/MuClient.Tui/FreezeBarRenderer.cssrc/MuClient.Tui/Glyphs.cssrc/MuClient.Tui/GmcpStats.cssrc/MuClient.Tui/KeypadScreenRenderer.cssrc/MuClient.Tui/MarkupFormatter.cssrc/MuClient.Tui/MuClient.Tui.csprojsrc/MuClient.Tui/MuGlyphApp.cssrc/MuClient.Tui/OptionsScreenRenderer.cssrc/MuClient.Tui/Powerline.cssrc/MuClient.Tui/Program.cssrc/MuClient.Tui/RailRenderer.cssrc/MuClient.Tui/SettingsOverlay.cssrc/MuClient.Tui/TabTitles.cssrc/MuClient.Tui/TimersScreenRenderer.cssrc/MuClient.Tui/TriggersScreenRenderer.cssrc/MuClient.Tui/Views/CommandInput.cssrc/MuClient.Tui/Views/OutputView.cssrc/MuClient.Tui/WorldsScreenRenderer.cstests/MuClient.Core.Tests/Automation/TriggerEngineTests.cstests/MuClient.Core.Tests/Commands/CommandCatalogTests.cstests/MuClient.Core.Tests/Commands/CommandMatcherTests.cstests/MuClient.Core.Tests/Configuration/ConfigurationTests.cstests/MuClient.Core.Tests/Input/InputHistoryTests.cstests/MuClient.Core.Tests/Text/MetersAndStatusTests.cstests/MuClient.Core.Tests/Workspace/DropZonesTests.cstests/MuClient.Core.Tests/Workspace/LayoutSolverTests.cstests/MuClient.Core.Tests/Workspace/PaneCommandsTests.cstests/MuClient.Core.Tests/Workspace/RailModelTests.cstests/MuClient.Core.Tests/Workspace/WorkspaceLayoutTests.cstests/MuClient.Core.Tests/Workspace/WorkspaceStateTests.cstests/MuClient.Core.Tests/Workspace/WorkspaceTests.cstests/MuClient.Tui.Tests/AliasesScreenRendererTests.cstests/MuClient.Tui.Tests/CaptureLineRendererTests.cstests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cstests/MuClient.Tui.Tests/FreezeBarRendererTests.cstests/MuClient.Tui.Tests/KeypadScreenRendererTests.cstests/MuClient.Tui.Tests/MarkupFormatterTests.cstests/MuClient.Tui.Tests/MuClient.Tui.Tests.csprojtests/MuClient.Tui.Tests/OptionsScreenRendererTests.cstests/MuClient.Tui.Tests/PowerlineTests.cstests/MuClient.Tui.Tests/RailRendererTests.cstests/MuClient.Tui.Tests/TabTitlesTests.cstests/MuClient.Tui.Tests/TimersScreenRendererTests.cstests/MuClient.Tui.Tests/TriggersScreenRendererTests.cstests/MuClient.Tui.Tests/WorldsScreenRendererTests.cstools/ansi_frame_to_image.pytools/fonts/README.mdtools/make-screenshots.sh
💤 Files with no reviewable changes (3)
- src/MuClient.Tui/ColorMapper.cs
- src/MuClient.Tui/Views/OutputView.cs
- src/MuClient.Tui/Views/CommandInput.cs
🚧 Files skipped from review as they are similar to previous changes (12)
- src/MuClient.Core/Configuration/TriggerSet.cs
- MuGlyph.slnx
- src/MuClient.Core/Configuration/CharacterDefinition.cs
- src/MuClient.Tui/GmcpStats.cs
- Directory.Packages.props
- .github/workflows/ci.yml
- src/MuClient.Core/Workspace/LayoutNode.cs
- src/MuClient.Core/Configuration/ConfigurationMigrator.cs
- src/MuClient.Core/Configuration/AppConfiguration.cs
- tests/MuClient.Core.Tests/Configuration/ConfigurationTests.cs
- tests/MuClient.Core.Tests/Workspace/WorkspaceLayoutTests.cs
- src/MuClient.Core/Configuration/WorldDefinition.cs
| @@ -0,0 +1,91 @@ | |||
| using MuClient.Core.Workspaces; | |||
|
|
|||
| namespace MuClient.Core.Commands; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Use file-scoped namespaces consistently.
src/MuClient.Core/Commands/CommandCatalog.cs#L3-L3: change tonamespace MuClient.Core.Commands;and remove the namespace-closing brace.src/MuClient.Core/Text/StatusFormatter.cs#L1-L1: change tonamespace MuClient.Core.Text;and remove the namespace-closing brace.src/MuClient.Tui/TabTitles.cs#L3-L3: change tonamespace MuClient.Tui;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/TimersScreenRendererTests.cs#L5-L5: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/TriggersScreenRendererTests.cs#L6-L6: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/WorldsScreenRendererTests.cs#L6-L6: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/AliasesScreenRendererTests.cs#L5-L5: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/CaptureLineRendererTests.cs#L3-L3: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cs#L4-L4: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/KeypadScreenRendererTests.cs#L4-L4: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.tests/MuClient.Tui.Tests/RailRendererTests.cs#L5-L5: change tonamespace MuClient.Tui.Tests;and remove the namespace-closing brace.
As per coding guidelines, **/*.cs must use file-scoped namespaces.
📍 Affects 11 files
src/MuClient.Core/Commands/CommandCatalog.cs#L3-L3(this comment)src/MuClient.Core/Text/StatusFormatter.cs#L1-L1src/MuClient.Tui/TabTitles.cs#L3-L3tests/MuClient.Tui.Tests/TimersScreenRendererTests.cs#L5-L5tests/MuClient.Tui.Tests/TriggersScreenRendererTests.cs#L6-L6tests/MuClient.Tui.Tests/WorldsScreenRendererTests.cs#L6-L6tests/MuClient.Tui.Tests/AliasesScreenRendererTests.cs#L5-L5tests/MuClient.Tui.Tests/CaptureLineRendererTests.cs#L3-L3tests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cs#L4-L4tests/MuClient.Tui.Tests/KeypadScreenRendererTests.cs#L4-L4tests/MuClient.Tui.Tests/RailRendererTests.cs#L5-L5
🤖 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/MuClient.Core/Commands/CommandCatalog.cs` at line 3, Convert the
namespace declarations to file-scoped form and remove each corresponding closing
brace: update src/MuClient.Core/Commands/CommandCatalog.cs (3-3),
src/MuClient.Core/Text/StatusFormatter.cs (1-1), and
src/MuClient.Tui/TabTitles.cs (3-3), plus
tests/MuClient.Tui.Tests/TimersScreenRendererTests.cs (5-5),
TriggersScreenRendererTests.cs (6-6), WorldsScreenRendererTests.cs (6-6),
AliasesScreenRendererTests.cs (5-5), CaptureLineRendererTests.cs (3-3),
CommandSurfaceRendererTests.cs (4-4), KeypadScreenRendererTests.cs (4-4), and
RailRendererTests.cs (5-5). Preserve all existing members while ensuring every
affected C# file uses a file-scoped namespace.
Source: Coding guidelines
Comment on lines
+106
to
+124
| def apply_sgr(seq, fg, bg, bold): | ||
| codes = [int(x) if x else 0 for x in seq.split(";")] or [0] | ||
| k = 0 | ||
| while k < len(codes): | ||
| c = codes[k] | ||
| if c == 0: | ||
| fg, bg, bold = DEFAULT_FG, DEFAULT_BG, False | ||
| elif c == 1: | ||
| bold = True | ||
| elif c == 22: | ||
| bold = False | ||
| elif c == 38 and k + 4 < len(codes) and codes[k + 1] == 2: | ||
| fg = (codes[k + 2], codes[k + 3], codes[k + 4]) | ||
| k += 4 | ||
| elif c == 48 and k + 4 < len(codes) and codes[k + 1] == 2: | ||
| bg = (codes[k + 2], codes[k + 3], codes[k + 4]) | ||
| k += 4 | ||
| k += 1 | ||
| return fg, bg, bold |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
apply_sgr only understands truecolor; 256-colour and default-colour codes are silently mis-parsed.
38;5;<n> / 48;5;<n> fall through the truecolor branch, so 5 and <n> get re-interpreted as standalone SGR codes — ESC[38;5;0m would hit the c == 0 reset branch and wipe all attributes. 39/49 (default fg/bg) and the 30-37/40-47/90-97 basic palette are also ignored, leaving stale colours. Fine if the headless driver only ever emits truecolor, but a small guard makes the tool robust against a driver change.
🛡️ Proposed hardening
elif c == 38 and k + 4 < len(codes) and codes[k + 1] == 2:
fg = (codes[k + 2], codes[k + 3], codes[k + 4])
k += 4
+ elif c == 38 and k + 2 < len(codes) and codes[k + 1] == 5:
+ k += 2 # 256-colour: skip, unsupported
+ elif c == 39:
+ fg = DEFAULT_FG
elif c == 48 and k + 4 < len(codes) and codes[k + 1] == 2:
bg = (codes[k + 2], codes[k + 3], codes[k + 4])
k += 4
+ elif c == 48 and k + 2 < len(codes) and codes[k + 1] == 5:
+ k += 2
+ elif c == 49:
+ bg = DEFAULT_BG📝 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.
Suggested change
| def apply_sgr(seq, fg, bg, bold): | |
| codes = [int(x) if x else 0 for x in seq.split(";")] or [0] | |
| k = 0 | |
| while k < len(codes): | |
| c = codes[k] | |
| if c == 0: | |
| fg, bg, bold = DEFAULT_FG, DEFAULT_BG, False | |
| elif c == 1: | |
| bold = True | |
| elif c == 22: | |
| bold = False | |
| elif c == 38 and k + 4 < len(codes) and codes[k + 1] == 2: | |
| fg = (codes[k + 2], codes[k + 3], codes[k + 4]) | |
| k += 4 | |
| elif c == 48 and k + 4 < len(codes) and codes[k + 1] == 2: | |
| bg = (codes[k + 2], codes[k + 3], codes[k + 4]) | |
| k += 4 | |
| k += 1 | |
| return fg, bg, bold | |
| def apply_sgr(seq, fg, bg, bold): | |
| codes = [int(x) if x else 0 for x in seq.split(";")] or [0] | |
| k = 0 | |
| while k < len(codes): | |
| c = codes[k] | |
| if c == 0: | |
| fg, bg, bold = DEFAULT_FG, DEFAULT_BG, False | |
| elif c == 1: | |
| bold = True | |
| elif c == 22: | |
| bold = False | |
| elif c == 38 and k + 4 < len(codes) and codes[k + 1] == 2: | |
| fg = (codes[k + 2], codes[k + 3], codes[k + 4]) | |
| k += 4 | |
| elif c == 38 and k + 2 < len(codes) and codes[k + 1] == 5: | |
| k += 2 # 256-colour: skip, unsupported | |
| elif c == 39: | |
| fg = DEFAULT_FG | |
| elif c == 48 and k + 4 < len(codes) and codes[k + 1] == 2: | |
| bg = (codes[k + 2], codes[k + 3], codes[k + 4]) | |
| k += 4 | |
| elif c == 48 and k + 2 < len(codes) and codes[k + 1] == 5: | |
| k += 2 | |
| elif c == 49: | |
| bg = DEFAULT_BG | |
| k += 1 | |
| return fg, bg, bold |
🤖 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 `@tools/ansi_frame_to_image.py` around lines 106 - 124, Extend apply_sgr to
consume 256-colour sequences 38;5;<n> and 48;5;<n> as complete parameters,
preventing their components from being processed as standalone SGR codes. Handle
39 and 49 by restoring DEFAULT_FG and DEFAULT_BG, and map basic
foreground/background palette codes, including bright variants, to their
corresponding colours while preserving existing truecolor and attribute
behavior.
- Cap the UI-side per-window line buffer at ScrollbackLines (was unbounded), shifting freeze points down when trimming. - CloseActiveWindow now clears _lines/_freezePoints so a reopened same-id spawn doesn't resurrect stale scrollback or a stale freeze split. - Percent-encode [ and ] in hyperlink targets before embedding in [link=…] so remote MXP/Pueblo/HTML can't close the tag early and inject markup. - Move mode: an unmapped a–j letter no longer clears the chosen target pane. - WorkspaceState.Restore rejects unknown layout-node types instead of silently degrading them to panes. - Drop the unreachable empty-tag branch in MarkupFormatter.StyleTag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Renderers: escape checkbox brackets ([[x]]/[[ ]]) in the trigger/timer/alias/ options screens so they render literally instead of being parsed as markup; pad the timer left column to a fixed visible width; surrogate-safe rail initials (EnumerateRunes); pad Worlds character rows on visible width. Core: CommandMatcher.FuzzySpread now returns the tightest subsequence span; Terminal.Gui→SharpConsoleUI doc comments (Workspace/LayoutSolver/PaneCommands/ WorkspaceLayout). Demo 'wtf' alias captures its argument. Tests: escaped-markup expectations, order-sensitive command-order assertion, TUnit Length().IsEqualTo. Tooling: ansi_frame_to_image.py hardens apply_sgr (256-colour/default), parses --html as a flag not a path, fixes the docstring name; bundle OFL.txt license pointer. Docs: refresh stale Terminal.Gui / px-vs-cols references. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The input row now reads as one solid full-width band. PromptControl fills its field background to the right edge on its own, but it measures to content width, so the band stopped mid-row; pin the control's Width to the window (re-synced on resize and before snapshot capture) so the field spans the row, and paint the prompt label with the same background (PromptMarkup) so the left edge joins the band with no gap. While wiring this, fix a regression: _inputRow was declared and passed to AddControl but never built (null), so the input PromptControl was never added to the window. Add _input directly and drop the dead field. Bundle the full SIL OFL 1.1 text (OFL.txt) and the Nerd Fonts license (LICENSE-NerdFonts.txt) alongside the screenshot font, as OFL 1.1 requires the license text to travel with a redistributed subset; a link does not satisfy it. Reference both from the fonts README. Ignore Python __pycache__. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Output panes stopped short of the console's right edge: the tab-header rule and content ended mid-row with empty space beyond. Root cause (confirmed against SharpConsoleUI source + docs/patterns.md): the control default HorizontalAlignment is Left, so the workspace HorizontalGrid, the pane TabControls, the split grids, and the spawn/frozen content grids all self-sized to content inside their slot instead of stretching to fill it. Set WithAlignment(HorizontalAlignment.Stretch) on each — matching the documented sidebar+content and Tabs (§12) patterns — so the Flex/Star pane column now claims the full width and the pane fills to the edge. Also split the bottom status bar like the header: the connection identity (● name · state) stays pinned left while the keepalive sparkline, host/encoding, char count, and palette hint form a cluster right-aligned to the far edge. Verified via headless snapshots (default/split/freeze/spawn/history all fill to col 120) and 310 Core + 90 Tui tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
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 (2)
CLAUDE.md (2)
84-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the remaining M1 instructions to match the new stack.
This table now correctly says TUnit and SharpConsoleUI, but the M1 checklist below still refers to xUnit test projects, Terminal.Gui, and only four projects. Those instructions are now contradictory and can mislead future contributors.
🤖 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 84 - 85, Update the M1 checklist in CLAUDE.md to consistently reference TUnit test projects, SharpConsoleUI instead of Terminal.Gui, and all five projects including Tui. Remove or revise the outdated xUnit, Terminal.Gui, and four-project references while preserving the checklist’s intended workflow.
99-101: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the obsolete TelnetNegotiationCore 1.0.0 dependency note.
CLAUDE.mdstill saysTelnetNegotiationCore 1.0.0only provides basic negotiation and ignores MCCP, MSDP, MXP, Pueblo, and ANSI, but the repo uses2.5.3and handles those capabilities. Update this note to the current package behavior, or remove it to avoid later confusion.🤖 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 99 - 101, Remove the obsolete TelnetNegotiationCore 1.0.0 dependency note from CLAUDE.md, or update it to accurately describe the repository’s current 2.5.3 usage and supported MCCP, MSDP, MXP, Pueblo, and ANSI capabilities.
🤖 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 `@tools/fonts/LICENSE-NerdFonts.txt`:
- Line 5: Correct the spelling in the Nerd Fonts license notice by changing
“explict” to “explicit,” leaving the rest of the notice unchanged.
---
Outside diff comments:
In `@CLAUDE.md`:
- Around line 84-85: Update the M1 checklist in CLAUDE.md to consistently
reference TUnit test projects, SharpConsoleUI instead of Terminal.Gui, and all
five projects including Tui. Remove or revise the outdated xUnit, Terminal.Gui,
and four-project references while preserving the checklist’s intended workflow.
- Around line 99-101: Remove the obsolete TelnetNegotiationCore 1.0.0 dependency
note from CLAUDE.md, or update it to accurately describe the repository’s
current 2.5.3 usage and supported MCCP, MSDP, MXP, Pueblo, and ANSI
capabilities.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 87b88ac0-e140-47a0-98c6-f3b517ba4a8b
📒 Files selected for processing (28)
.gitignoreCLAUDE.mddocs/design/README.mdsrc/MuClient.Core/Commands/CommandMatcher.cssrc/MuClient.Core/Workspace/LayoutSolver.cssrc/MuClient.Core/Workspace/PaneCommands.cssrc/MuClient.Core/Workspace/Workspace.cssrc/MuClient.Core/Workspace/WorkspaceLayout.cssrc/MuClient.Core/Workspace/WorkspaceState.cssrc/MuClient.Tui/AliasesScreenRenderer.cssrc/MuClient.Tui/DemoScene.cssrc/MuClient.Tui/MarkupFormatter.cssrc/MuClient.Tui/MuGlyphApp.cssrc/MuClient.Tui/OptionsScreenRenderer.cssrc/MuClient.Tui/RailRenderer.cssrc/MuClient.Tui/TimersScreenRenderer.cssrc/MuClient.Tui/TriggersScreenRenderer.cssrc/MuClient.Tui/WorldsScreenRenderer.cstests/MuClient.Core.Tests/Text/MetersAndStatusTests.cstests/MuClient.Tui.Tests/AliasesScreenRendererTests.cstests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cstests/MuClient.Tui.Tests/OptionsScreenRendererTests.cstests/MuClient.Tui.Tests/TimersScreenRendererTests.cstests/MuClient.Tui.Tests/TriggersScreenRendererTests.cstools/ansi_frame_to_image.pytools/fonts/LICENSE-NerdFonts.txttools/fonts/OFL.txttools/fonts/README.md
🚧 Files skipped from review as they are similar to previous changes (23)
- .gitignore
- tools/fonts/README.md
- src/MuClient.Tui/TimersScreenRenderer.cs
- src/MuClient.Core/Workspace/PaneCommands.cs
- tests/MuClient.Tui.Tests/CommandSurfaceRendererTests.cs
- src/MuClient.Tui/DemoScene.cs
- src/MuClient.Tui/TriggersScreenRenderer.cs
- src/MuClient.Tui/WorldsScreenRenderer.cs
- src/MuClient.Tui/MarkupFormatter.cs
- tests/MuClient.Core.Tests/Text/MetersAndStatusTests.cs
- src/MuClient.Tui/RailRenderer.cs
- tests/MuClient.Tui.Tests/TimersScreenRendererTests.cs
- src/MuClient.Tui/AliasesScreenRenderer.cs
- tests/MuClient.Tui.Tests/TriggersScreenRendererTests.cs
- docs/design/README.md
- src/MuClient.Core/Workspace/Workspace.cs
- tests/MuClient.Tui.Tests/AliasesScreenRendererTests.cs
- tests/MuClient.Tui.Tests/OptionsScreenRendererTests.cs
- src/MuClient.Core/Workspace/LayoutSolver.cs
- src/MuClient.Core/Workspace/WorkspaceLayout.cs
- src/MuClient.Tui/MuGlyphApp.cs
- src/MuClient.Core/Commands/CommandMatcher.cs
- src/MuClient.Core/Workspace/WorkspaceState.cs
|
|
||
| There are various sources used under various licenses: | ||
|
|
||
| * Nerd Fonts source fonts, patched fonts, and folders with explict OFL SIL files are licensed under SIL OPEN FONT LICENSE Version 1.1 (see below). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the spelling in the license notice.
Change explict to explicit.
Proposed fix
-* Nerd Fonts source fonts, patched fonts, and folders with explict OFL SIL files are licensed under SIL OPEN FONT LICENSE Version 1.1 (see below).
+* Nerd Fonts source fonts, patched fonts, and folders with explicit OFL SIL files are licensed under SIL OPEN FONT LICENSE Version 1.1 (see below).📝 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.
Suggested change
| * Nerd Fonts source fonts, patched fonts, and folders with explict OFL SIL files are licensed under SIL OPEN FONT LICENSE Version 1.1 (see below). | |
| * Nerd Fonts source fonts, patched fonts, and folders with explicit OFL SIL files are licensed under SIL OPEN FONT LICENSE Version 1.1 (see below). |
🧰 Tools
🪛 LanguageTool
[grammar] ~5-~5: Ensure spelling is correct
Context: ... fonts, patched fonts, and folders with explict OFL SIL files are licensed under SIL OP...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_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 `@tools/fonts/LICENSE-NerdFonts.txt` at line 5, Correct the spelling in the
Nerd Fonts license notice by changing “explict” to “explicit,” leaving the rest
of the notice unchanged.
Source: Linters/SAST tools
Open the command surface (☰/⌃P) in the headless snapshot so the menu can be captured for docs — 'menu' over the single-pane workspace, 'menu-split' over a two-pane (main | Chat) split. The palette opens a normal modal window, which ForceRender paints over the resumed demo scene, same as the settings screens. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The M1 first-task checklist still named four projects, xUnit, and a Terminal.Gui window, and the dependency note still described TelnetNegotiationCore 1.0.0 — both contradicting the doc's own Locked Decisions and Repository State. Mark M1 as delivered and correct it to TUnit + SharpConsoleUI; update the TNC note to 2.5.3 (which negotiates MCCP/MSDP/MXP itself, while Pueblo and payload parsing stay our layer). The 'explict' typo CodeRabbit flagged in tools/fonts/LICENSE-NerdFonts.txt is left as-is: it's a verbatim copy of the upstream Nerd Fonts license, and bundled third-party license text should match the canonical source, typos included. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The command surface (☰/⌃P) rendered with full window chrome — minimize/maximize/ close buttons and a corner resize grip — and a fixed 66×18 size that was too short for the catalog, so it showed a scrollbar. Use the framework's own affordances to make it a clean overlay: HideTitleButtons() drops the [_][+][X] controls and Resizable(false) drops the resize grip (Esc still closes). Size the window to fit the full unfiltered catalog (search row + all command lines + border), capped to the usable desktop, so nothing scrolls. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The surface was a fixed 66 wide while its rows used ~45, leaving a dead gutter on the right (where the scrollbar used to sit) and a selection highlight that only wrapped the title. Size the window width to the widest rendered row (capped to the desktop, same as the height fit) so the surface hugs its content, and pad the selected row's accent highlight to the full inner width so it reads as one continuous selection bar across title + subtitle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Rework F5 into a full-console layout: a header band (title + keyboard hints), a two-column WORLDS-list / world-detail body, a full-width CHARACTER editing pane on its own elevated background, and a footer action bar (Cancel/Save) pinned to the bottom row. Add colour throughout — teal section accents, per-world accent spines, light values against dim labels, an accent Save button. The renderer now takes width/height (0 = natural mode, still used by the unit tests) and, when given the console size, fills it: full-width bands, the edit-pane background, and the footer at the last row. Make the settings overlay host a clean full-screen surface: frameless (no title bar, buttons, or resize grip), a deep panel background, and the panel stretched so that background fills the console. All F2–F9 screens ride on this cleaner host; F5 passes the desktop dimensions so it lays out edge-to-edge. 90 Tui tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Rebuild the F5 editing pane on the framework's grid layout instead of hand-merged markup columns. WorldsScreenRenderer now exposes each region as a pure markup block (header, worlds list, detail, character form, trigger checklist, footer); a new WorldsScreenView composes them into controls: a root grid (header band / body / edit pane / footer) with the editing pane a HorizontalGrid whose left panel holds the character form (left-aligned) and whose right panel holds the assigned trigger sets — pushed right by a flex spacer so the block sits on the right while its [x] checkboxes stay left-aligned. The edit grid's own background gives the full-width elevated band. Footer Cancel/Save stays pinned to the last row; header/footer/edit bands all span the full console. The settings overlay now hosts an arbitrary control (frameless full-screen modal, deep panel background); markup screens (F2–F4/F6–F9) are wrapped in a stretched panel as before, while F5 supplies the composed control tree. Render() still merges the same blocks for the unit tests. 90 Tui tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
The footer band (world/character position + Cancel/Save) sat directly against the editing-pane band, so "character 1/2" read as if it belonged to the character setup section. Insert a one-row panel-background gap between the editing pane and the footer so the footer reads as its own bar. Three distinct bands now: edit pane, gap, footer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Mechanical rename of all five source projects and their five test projects, plus the solution file. No behavioural change: 514 tests pass before and after. The repo previously carried three names for one product -- MuGlyph (repo/product), MuClient.* (assemblies) and muglyph (binary). This collapses the assembly axis onto the new product name; the brand strings follow in the next commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers every human-visible surface: product name, README (now positioning the client against SharpMUSH and SharpClient), CLAUDE.md, docs, the release workflow's artifact names, and the design reference. Behavioural renames worth calling out: - binary: muglyph -> sharpmuterm - config dir: ~/.config/MuGlyph -> ~/.config/SharpMUTerm - env vars: MUGLYPH_GRAPHICS/MUGLYPH_SIXEL -> SHARPMUTERM_* - User-Agent: MuGlyph/0.1 -> SharpMUTerm/0.1, pointing at the new repo - bundled subset font: MuGlyphMonoNerd.woff -> SharpMUTermMonoNerd.woff - header ribbon brand chip: "glyph" -> "muterm" No published releases exist, so none of these need compatibility shims. Also corrected three stale docs found in passing: the test count (391 -> 514), a claim that CI runs `dotnet test` (it runs `dotnet run` per test project, since TUnit on MTP dropped VSTest support in .NET 10), and CLAUDE.md contradicting itself on the same point. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Records the criterion the name had to meet ("pay its own rent" -- the
name alone says what the thing is, no tagline), the alternatives weighed
and why each lost, the resulting surface, and the follow-ups this rename
deliberately does not cover.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A detailed handoff doc for the next session — outstanding backlog (panel treatment for the other config screens, graphics/Kitty wiring, live config editing, real-terminal verification) and the hard-won gotchas (TUnit test invocation, the snapshot→SVG pipeline and its Chromium clipping trap, SharpConsoleUI alignment/fill behavior, TelnetNegotiationCore 2.5.3, the UI-agnostic-Core rule, and the intentional review skips). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015w2WJ5M22CZmg9md67zpeB
Rename MuGlyph to SharpMUTerm
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.
Advances MuGlyph into milestone M5 (full parity & polish) on top of the merged M1–M4 foundation, and clears the deferred review items from PR #1.
Status
Releasebuild (0 warnings). 299 tests pass — Core 200, Graphics 57, Scripting 42.Protocols & text (Core, UI-agnostic, tested)
MxpParserandPuebloParser— incremental, line-oriented parsers (same shape asAnsiParser, unified behind a newILineParser) turning markup into styled spans: formatting, colours (viaWebColors), entities, line breaks, and clickable<SEND>/<A>links & commands. (44 MXP + 32 Pueblo tests.)SpanInteraction— an optional click behaviour (send-command / hyperlink) onStyledSpan, produced by the parsers and realised by the UI.EmojiSubstitutor— opt-in emoticon (:)→ 🙂) and:shortcode:(:fire:→ 🔥) substitution.WorldDefinitiongainsContentFormat(Ansi/Mxp/Pueblo) andEmojisettings;WorldSessionselects the parser and applies emoji per world.TUI integration
OutputViewtracks per-cell interactions and activates a command/link span on click;MuGlyphAppsends the command / opens the link, shows a GMCP-driven stat line, and captures spawn-routed output.Deferred review fixes (from PR #1's CodeRabbit review)
TelnetSession: clean up the transport on connect failure, clear the interpreter on disconnect (so reconnect works), and honour cancellation on send.WorldSession: dispose a prior session before reconnecting.KittyGraphicsProtocol: reject placeholder image ids above 24 bits instead of truncating.ScriptException: also parse the[source]:LINE:runtime-error format.Packaging
linux-x64,win-x64) and a tagged release workflow (docs/PACKAGING.md). The single-file knobs live only in the publish profiles, so ordinarydotnet build/runand CI never require a runtime identifier.Roadmap note (per discussion)
docs/PLAN.mdrecords the still-open M5+ items (dedicated spawn/input windows, puppets, map view) and a concrete plan for an in-TUI web view (AngleSharp text render reusingSpanInteraction,<img>through the existing Kitty/Sixel/half-block pipeline, optional Playwright snapshot mode).Verification note
A headless environment can't visually verify a TUI or render Kitty graphics, so the Graphics and Tui layers are build-verified; all protocol/parser/engine logic lives in
MuClient.Coreand is covered by tests.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit