Skip to content

Update docs/HANDOFF.md to current state - #5

Merged
HarryCordewener merged 2 commits into
mainfrom
docs/handoff-sharpmuterm-refs
Jul 28, 2026
Merged

Update docs/HANDOFF.md to current state#5
HarryCordewener merged 2 commits into
mainfrom
docs/handoff-sharpmuterm-refs

Conversation

@HarryCordewener

@HarryCordewener HarryCordewener commented Jul 28, 2026

Copy link
Copy Markdown
Member

docs/HANDOFF.md arrived on main via PR #2. It was written earlier and never updated, so it
directed whoever picks up the work next to paths that no longer exist:

  • dotnet build MuGlyph.slnx
  • src/MuClient.Tui/MuGlyphApp.cs
  • dotnet run --project tests/MuClient.Core.Tests
  • dotnet run --project src/MuClient.Tui

Following the document as written fails at the first command.

Changes

  • Every path, command, and project name corrected so they resolve.
  • Header block rewritten as plain current state — repository, branch to start from, test
    status. It previously described an unmerged branch and an open PR at a repo path that no
    longer exists, and told readers to go check whether that branch was still there.
  • Test count corrected to 514 across all five projects; it previously listed only Core + Tui
    (400).
  • "Key Files Touched This Session" → "Key Files" — the file→role table is orientation for
    the next session, not a changelog.
  • No historical framing anywhere. The document states what is true now; it does not narrate
    what changed to get here. The reasoning behind the naming lives in
    docs/superpowers/specs/2026-07-28-sharpmuterm-naming-design.md, which is where that belongs.

Verification

Every path referenced in the document was checked against the tree; all resolve. (docs/patterns.md
appears in the text but refers to a file in the upstream nickprotop/ConsoleEx repo, not this one.)

Still outstanding

This PR covers HANDOFF.md only. Four files on main still carry the old name, all from PR #3:

  • README.md — the <h1> title and logo alt text
  • assets/fonts/MuGlyph.ttf — filename, plus the family name baked into the font binary
  • assets/fonts/README.md
  • tools/make-glyph-font.py

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Updated handoff documentation to reflect the SharpMUTerm rename and repository/linkage details.
    • Revised build, testing, and visual verification instructions for the renamed solution and projects.
    • Updated architecture guidance and related file references to match the current project structure.
    • Clarified contribution workflow to require branching and pull requests instead of direct commits to main.

docs/HANDOFF.md was written before the rename and landed on main via
PR #2, so it still pointed future readers at paths that no longer exist:
`dotnet build MuGlyph.slnx`, `src/MuClient.Tui/MuGlyphApp.cs`, and the
`tests/MuClient.*` test projects. Following it would have failed
immediately.

Updates every path and command to the SharpMUTerm names, and replaces
the header block -- which described an unmerged branch and PR at the old
repo -- with the current state: both PRs merged, work starts fresh off
main. Test count corrected to the full 514 across five projects (it
listed only Core and Tui).

Verified every path the document names now resolves in the tree.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

docs/HANDOFF.md was updated to replace MuGlyph/MuClient references with SharpMUTerm names across project notes, build and verification commands, architecture guidance, workflow instructions, and touched-file paths.

Changes

Handoff documentation

Layer / File(s) Summary
Project naming and build guidance
docs/HANDOFF.md
Project references, task wording, test commands, visual verification commands, and architecture rules now use SharpMUTerm names.
Workflow and touched-file references
docs/HANDOFF.md
Branching guidance and listed TUI paths now reflect the renamed project structure.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the doc-only update to HANDOFF.md and its refreshed current-state content.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

A handoff should describe the current state only -- the next session
needs to know what is true now, not what changed to get here.

Removes the rename/repo-move note, retitles "Key Files Touched This
Session" to "Key Files" (the file->role map is orientation, not
history), and rewrites the header block as plain current state:
repository, where to branch from, test status.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@HarryCordewener HarryCordewener changed the title Fix stale MuGlyph references in docs/HANDOFF.md Update docs/HANDOFF.md to current state Jul 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@docs/HANDOFF.md`:
- Line 117: In docs/HANDOFF.md, add one blank line immediately after the closing
fenced test-command block and before the following prose to satisfy Markdownlint
MD031.
🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d21f2da-5583-4457-b6f8-3f6e951e6b6e

📥 Commits

Reviewing files that changed from the base of the PR and between fa8479c and 90333ac.

📒 Files selected for processing (1)
  • docs/HANDOFF.md

Comment thread docs/HANDOFF.md
dotnet run --project tests/MuClient.Tui.Tests </dev/null
dotnet run --project tests/SharpMUTerm.Core.Tests </dev/null
dotnet run --project tests/SharpMUTerm.Tui.Tests </dev/null
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add a blank line after the fenced test commands.

The closing fence on Line 117 is immediately followed by prose on Line 118, triggering Markdownlint MD031. Insert one blank line after the fence.

🧰 Tools
🪛 markdownlint-cli2 (0.23.1)

[warning] 117-117: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/HANDOFF.md` at line 117, In docs/HANDOFF.md, add one blank line
immediately after the closing fenced test-command block and before the following
prose to satisfy Markdownlint MD031.

Source: Linters/SAST tools

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/HANDOFF.md (1)

107-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document how to run all five test suites.

The commands shown execute only Core and Tui, while the next line says the primary signal includes Core, Graphics, Scripting, Web, and Tui. Add commands for the remaining three suites, or explicitly label these as a partial smoke-test subset and provide the full five-suite procedure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/HANDOFF.md` around lines 107 - 112, Update the test-running instructions
near the existing Core and Tui commands to document all five suites: Core,
Graphics, Scripting, Web, and Tui, each with the same stdin-detaching behavior.
Alternatively, clearly label the current commands as a partial smoke-test subset
and add the complete five-suite procedure, keeping the stated build and
warning-free expectations consistent.
🤖 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.

Outside diff comments:
In `@docs/HANDOFF.md`:
- Around line 107-112: Update the test-running instructions near the existing
Core and Tui commands to document all five suites: Core, Graphics, Scripting,
Web, and Tui, each with the same stdin-detaching behavior. Alternatively,
clearly label the current commands as a partial smoke-test subset and add the
complete five-suite procedure, keeping the stated build and warning-free
expectations consistent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7d93a654-1244-4719-b71e-775d4e6cc5b1

📥 Commits

Reviewing files that changed from the base of the PR and between 90333ac and fe19615.

📒 Files selected for processing (1)
  • docs/HANDOFF.md

@HarryCordewener
HarryCordewener merged commit caa0970 into main Jul 28, 2026
3 checks passed
@HarryCordewener
HarryCordewener deleted the docs/handoff-sharpmuterm-refs branch July 28, 2026 20:15
HarryCordewener added a commit that referenced this pull request Jul 31, 2026
"I am looking for the characters to have different numbers? Am I not
communicating something right here? Like, I would expect Riko's next window
to be #5 for instance. But perhaps I should attack this a different way.
Let's assume we keep this way. Pane 1-2-3-4- etc under the same connection.
Let's create a different mechanic to easily be able to switch characters
then!" - and then, choosing between the two: "Let's take option B for
cycling characters & panes separately."

Global window numbering failed the first day it was used. With three
characters sharing pane 1 as tabs, every character's row read Alt-1, because
their windows happened to be numbered from one run; and nine digits do not
stretch over everybody's windows - six across three characters already
crowds them.

Alt+1-9 now selects a window of the *active character*, numbered from 1
within that character. Alt-1 is your own window whoever you are, Alt-2 your
first capture. An unowned window (the web view) is in every character's
list, so it wears a different digit under each - which is exactly the set
the rail draws window rows for, so the sidebar and the chord are one list
read twice.

Characters are reached by Alt+J and Alt+K, forward and back. Letters and not
a third digit row because there is no third digit-bearing modifier this
terminal delivers: read off a pty, kitty writes Alt+Shift+1 as CSI 49;4u and
Ctrl+Shift+N as CSI 110;6u - kitty-keyboard-protocol sequences the parser
does not decode and drops - while Alt+j and Alt+k are a plain ESC j / ESC k.
The cycle walks only the characters already open, because SwitchToCharacter
*creates* a session and a window for one that is not, and a cycle key that
did that per press would dial through a configuration by accident.

The sidebar follows. A character row carries Alt-J or Alt-K when it is one
of the two neighbours and nothing otherwise - including the row you are on,
whose marker already says so. It used to carry the chord of that character's
own window, which under a scoped numbering is Alt-1 against everybody.

The chord now leads each row instead of trailing it. "There is still way too
much room after a window's name before it hits 'alt-1'" - that gap was the
reserved pen and unread fields, which cannot be removed (a cell that costs
only when it has something to say resizes the sidebar, and the sidebar's
width comes out of the panes, which every server is told over NAWS). So the
chord moved to the front, against the name it names, and the badges ended up
at the right edge. Measured: the demo rail is 22 columns before and after,
and the gap is 5 cells before and 0 after. The field is reserved per row
*kind*, because with fewer than two characters open no character row can
hold a cycle chord and reserving across both spent three cells on every
character row of the commonest client there is.

Alt where Alt is safer, measured rather than preferred:

- Disconnect moves from Ctrl+D to Alt+D, so it and Alt+R read as one pair.
  Ctrl+D is released outright rather than kept as an alias - a second key for
  one action is either a secret or a duplicate row on every surface that
  lists chords - and releasing it hands a clean Ctrl chord back for macros.
  Nothing takes it: HandleMoveInput is gated on IsMovable (false here) and
  acts only on the arrows and X, and InputBarControl's Ctrl table has no D.
  Pinned by a test that presses it and checks the connection and the window
  count.
- Ctrl+Tab is removed, and it was never real. A terminal writes 0x09 for it,
  which *is* Tab, so the parser reports ConsoleKey.Tab with no Control bit
  and the claim could never have matched - while Claimed is consulted first,
  so F4 told users a chord was taken that cannot arrive. The byte is in
  ControlBytes now, so Verdict says so.
- Deliberately left on Ctrl: Ctrl+R (readline's history search), Ctrl+P,
  Ctrl+Q (safe here - TerminalRawMode clears IXON, so it is not XON),
  Ctrl+B, Ctrl+O, Ctrl+N/W/F and the command line's A/E/K/U/L. A sweep that
  moved everything would be as wrong as one that moved nothing.
- Flagged, not fixed: Ctrl+N and Ctrl+O have no reverse, and Ctrl+W and
  Ctrl+B x are two chords for one action. Both are behaviour changes rather
  than modifier moves.

Two fixes found on the way. BuildCharacterRefs passed `_active?.SessionKey ==
key` as *Connected*, which is a different question - and since the catalog
skips the focused character, every "Switch to ..." entry read `offline`
however many worlds were live. And main's TabExpansionTests asserted two
constants against their own literals, which TUnit's analyzer refuses; they
are behavioural now.

New `characters` snapshot view: two characters genuinely open, the one state
the Alt-J/Alt-K column can be seen in and the one thing `connections` cannot
fake.

Core 813, Graphics 83, Scripting 42, Web 37, Tui 1355. Warning-free.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GpL7Ht6sLBsSEtVNsYcXMM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant