feat(ui): show status bar icons beside their toggle keys in help - #352
Merged
Merged
Conversation
The help overlay listed every toggle key but never the glyph the status bar renders for it, so the icon legend lived only in the README table. Each View and Search row now carries its glyph in front of the description, resolved through the same live keymap as the key column so custom bindings and chords stay correct. Rows in those sections without a glyph are indented to keep the description column straight. Related to #351
…teFile Split the fill-close-rename step into commitTemp behind a TempFile interface so a moq mock can fail the write or the close. Both paths now prove the temp file is removed and the target is left untouched. Package coverage goes from 70.6% to 100%.
…ction map statusModeIcons carried its own copy of the twelve glyphs, so an edit in one place and not the other would put the status bar and the help legend out of step. The status bar now reads each glyph from the map by action.
TempFile had no caller outside the package; it was exported only so a sibling mocks package could name the generated type. Follow the app/signal.go precedent instead: the interface stays unexported and moq writes temp_file_moq_test.go into fsutil itself.
There was a problem hiding this comment.
🟡 Changes recommended
Documentation wording added in multiple places incorrectly implies every status icon is “toggled” by a key, but some icons reflect state driven by actions (e.g. search/reviewed).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the TUI help overlay to show the same status-bar mode icons alongside their corresponding key bindings, keeping the legend and status bar in sync via a shared icon mapping. Also refactors fsutil.AtomicWriteFile to make previously unreachable cleanup branches testable, bringing app/fsutil to full coverage.
Changes:
- Centralize status bar glyphs in
statusIconForActionand render both status bar indicators and help overlay rows from that single mapping. - Add help-overlay formatting (icon + aligned indentation) plus a unit test pinning the icon/row behavior.
- Refactor
AtomicWriteFileby extracting the write/close/rename sequence behind an unexportedtempFileinterface and add tests for write/close failure cleanup.
File summaries
| File | Description |
|---|---|
| site/docs.html | Docs update noting the help overlay now shows status icons next to bindings. |
| README.md | Mirrors the docs update for the Status Bar Icons section. |
| plugins/codex/skills/revdiff/references/usage.md | Mirrors the docs update for the Codex skill usage reference. |
| .claude-plugin/skills/revdiff/references/usage.md | Mirrors the docs update for the Claude plugin usage reference. |
| app/ui/view.go | Introduces statusIconForAction and switches status bar icon rendering to use it. |
| app/ui/handlers.go | Prefixes help descriptions with icons (or aligned padding) based on action → icon mapping. |
| app/ui/handlers_test.go | Adds coverage to ensure help rows include icons/padding as intended. |
| app/fsutil/fsutil.go | Extracts commitTemp and adds tempFile interface + go:generate for a moq mock. |
| app/fsutil/fsutil_test.go | Adds tests covering temp-file cleanup on write/close failure paths. |
| app/fsutil/temp_file_moq_test.go | New generated moq mock used by the new failure-path tests. |
| .claude/rules/gotchas.md | Updates internal notes to reflect the centralized icon mapping and new help test. |
Review details
Files not reviewed (1)
- app/fsutil/temp_file_moq_test.go: Generated file
- Files reviewed: 10/11 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| - Horizontal scroll indicators (`«`/`»`): see `applyHorizontalScroll()` in `diffview.go`. `«` replaces first visible column when scrolled past hidden content. `»` extends 1 col into right padding. Bg split: `«` and separator space use line bg via `indicatorBg()`, `»` glyph uses `DiffBg`. Only in unwrapped mode. | ||
| - Vertical scrollbar thumbs: `applyScrollbar()` / `applyNavigationScrollbar()` in `app/ui/scrollbar.go` are **post-`lipgloss.Render()`** transforms on rendered pane strings (distinct from `padContentBg`, which runs *pre-render* on assembled multi-line content, and from `extendLineBg`/`applyHorizontalScroll`, which run *per line* before viewport assembly). They replace the right-border `│` with `┃` (heavy vertical, bold-wrapped via `\x1b[1m...\x1b[22m`) on rows that map to the visible viewport portion. Diff uses Bubble viewport state; navigation uses `sidepane.ScrollState` from file tree / markdown TOC after rendering. No-op when content fits, viewport height is zero, or the rendered pane's line count differs from the expected shape (defensive bail when wrapping breaks the layout invariant). The slice replacement preserves the surrounding ANSI envelope (border-fg + optional `BorderBackground`) because it swaps only the rune itself — prefix/suffix bytes are kept intact regardless of the thumb's added SGR wrap. Glyphs `┃` and `│` are both 1 cell wide so display geometry stays unchanged. `\x1b[22m` resets only intensity to keep the border bg intact (vs `\x1b[0m` which would kill it). Layout coupling: diff pane assumes top-border row, **single-line** header, `vh` viewport rows, bottom-border — diff rows start at `diffScrollbarFirstViewportRow = 2`; navigation pane assumes top-border row, content rows, bottom-border — navigation rows start at `navigationScrollbarFirstViewportRow = 1`. The diff single-line header invariant is enforced by `truncateHeaderTitle()` in `view.go` (filenames are sanitized via `style.SanitizeFilenameForDisplay` to drop control bytes, then left-truncated with `style.TruncateLeftToWidth` so lipgloss never soft-wraps the header). The `paneW` passed to `truncateHeaderTitle` MUST match the lipgloss `Width()` later applied to the pane — both branches of `View()` derive it via `width-2` (tree hidden) / `width-treeWidth-4` (two-pane). Changing one side without the other re-introduces the wrap regression. Any change to pane pre-content row counts (multi-line diff header, tree/TOC header, status pill above viewport, etc.) must update the matching scrollbar row offset in lockstep. | ||
| - Status bar mode icons: `▼◉↩≋⊟⊂#b±✓∅` rendered via `statusModeIcons()`. Graceful degradation drops segments on narrow terminals. | ||
| - Status bar mode icons: `▼⊂◉↩≋⊟#b±✓∅` rendered via `statusModeIcons()`. Graceful degradation drops segments on narrow terminals. The glyphs live in one place, `statusIconForAction` (same file, keyed by `keymap.Action`): the status bar reads them from there and the help overlay shows each beside its toggle key. A new icon needs an entry there plus an indicator row in `statusModeIcons`. `TestBuildHelpSpec_StatusIconsOnToggleRows` pins the help mapping. |
"toggles" was wrong for two rows: / opens the search prompt and the search icon lights on matches present, Space marks a file and the reviewed icon reflects that state.
Deploying revdiff with
|
| Latest commit: |
3440a4d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bd0c6755.revdiff.pages.dev |
| Branch Preview URL: | https://status-icon-legend.revdiff.pages.dev |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
the help overlay listed every toggle key but never the glyph the status bar renders for it, so the icon legend lived only in the README table. Each View and Search row now carries its glyph in front of the description, e.g.
v ▼ toggle collapsed view, resolved through the same live keymap as the key column so custom bindings and chords stay correct. Rows in those sections without a glyph are indented to keep the description column straight.the glyphs now live in one place,
statusIconForAction, and the status bar reads from it too, so the bar and the legend cannot drift apart.also on the branch:
app/fsutilcoverage was at 70.6% because the write and close failure cleanups ofAtomicWriteFilewere unreachable. The fill-close-rename step is split out behind an unexportedtempFileinterface with an in-package moq mock, and both paths are tested. Package coverage is 100%.Related to #351