Skip to content

feat(ui): add toggleable line numbers in diff gutterLine numbers#21

Merged
umputun merged 12 commits intoumputun:masterfrom
rashpile:line-numbers
Apr 5, 2026
Merged

feat(ui): add toggleable line numbers in diff gutterLine numbers#21
umputun merged 12 commits intoumputun:masterfrom
rashpile:line-numbers

Conversation

@rashpile
Copy link
Copy Markdown
Contributor

@rashpile rashpile commented Apr 5, 2026

Summary

Adds side-by-side old/new line number columns to the diff gutter, toggled at runtime with L key.

Closes #20

What's changed

  • New L key toggles line number display on/off
  • Two-column gutter shows old and new line numbers ( OOO NNN format)
  • Column width auto-adjusts per file based on max line number
  • Works across all render modes: normal, wrapped (w), and collapsed (v)
  • # status bar icon indicates when line numbers are active
  • Help overlay (?) documents the L keybinding

Implementation

  • lineNumbers bool and lineNumWidth int fields on the Model
  • lineNumGutter(dl) helper formats the gutter using muted ANSI (raw sequences, no lipgloss reset)
  • Gutter integrated into renderDiffLine, renderWrappedDiffLine, renderCollapsedAddLine, renderDeletePlaceholder
  • Width recomputed in handleFileLoaded and toggleLineNumbers
  • Guard for narrow terminals prevents negative content width in horizontal scroll

Test plan

  • Unit tests for toggle, width computation, gutter formatting
  • Integration tests for normal, wrapped, and collapsed modes with line numbers on/off
  • End-to-end toggle test verifying rendered output
  • Full test suite passes

Credits

powered by

  • claude code
  • ralphex

@rashpile rashpile requested a review from umputun as a code owner April 5, 2026 11:41
@rashpile
Copy link
Copy Markdown
Contributor Author

rashpile commented Apr 5, 2026

Line numbers with annotations icon:

CleanShot 2026-04-05 at 15 46 44

Copy link
Copy Markdown
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

the line numbers feature itself looks good, two-column old/new is the right approach (we had single-column before and removed it because it was confusing in diff mode).

the branch needs a rebase on master though - it's based on a commit before --all-files/--exclude was merged, so the diff shows those files as "removed." once rebased the diff should be clean.

couple things to fix:

  1. linter: var old, new string in diffview.go:30 - new shadows the Go builtin, rename to smth like oldCol/newCol
  2. linter: untagged switch in diffview.go:31 - use switch dl.ChangeType { case ... } instead of switch { case dl.ChangeType == ... }
  3. lineNumGutter comment says "raw ANSI" but uses m.styles.LineNumber.Render() (lipgloss). works fine in practice since the gutter is concatenated before content, but the comment is misleading - pls fix

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an optional old/new side-by-side line-number gutter to the diff view, toggleable at runtime with the L key, and updates rendering across normal/wrapped/collapsed modes plus documentation and tests.

Changes:

  • Introduce Model.lineNumbers + Model.lineNumWidth, compute width per file, and add L key handling + status bar indicator.
  • Prepend a two-column old/new line-number gutter to diff rendering (normal, wrapped, collapsed, delete placeholders), including width adjustments for wrapping and horizontal scroll.
  • Add unit/integration-style tests for toggling, width computation, gutter formatting, and rendered output; update help/README/plugin usage docs.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ui/model.go Adds model state + key handling for toggling and width recomputation on file load.
ui/model_test.go Adds tests covering toggle behavior, width computation, help/status indicators, and an end-to-end render check.
ui/diffview.go Implements gutter formatting and integrates it into normal/wrapped diff rendering and scroll/wrap width math.
ui/diffview_test.go New tests for gutter formatting and rendering with/without line numbers (incl. wrap).
ui/collapsed.go Integrates the gutter into collapsed-mode rendering paths and adjusts wrap/scroll calculations.
ui/collapsed_test.go Adds a collapsed-mode render test with line numbers enabled.
ui/annotate.go Adjusts delete-placeholder wrap-height accounting to include the extra gutter width.
README.md Documents the new L keybinding and feature in the key table/features list.
CLAUDE.md Updates internal notes/docs to include the new line-number mode and keybinding.
.claude-plugin/skills/revdiff/references/usage.md Updates plugin usage docs with the L toggle keybinding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rashpile added 12 commits April 5, 2026 22:35
- Fix horizontal scroll cut width not accounting for line number gutter
  width in renderDiffLine, renderCollapsedAddLine, renderDeletePlaceholder
- Fix cursorViewportY delete-placeholder wrap width ignoring line numbers
- Update CLAUDE.md with # status icon, L key dispatch, line number data flow
- Update README.md and plugin usage.md with L keybinding
Rename var old/new to oldCol/newCol to avoid shadowing Go builtin.
Use tagged switch on dl.ChangeType instead of untagged switch.
Fix comment: gutter uses lipgloss style, not raw ANSI.
Align with toggleWrapMode/toggleCollapsedMode by checking
m.focus == paneDiff before toggling, so pressing L while
the tree pane is focused is a no-op.
@rashpile
Copy link
Copy Markdown
Contributor Author

rashpile commented Apr 5, 2026

Rebased on latest master — diff is clean now.

Fixes:

  • var old, new → renamed to oldCol/newCol to avoid shadowing Go builtin new
  • untagged switch → changed to switch dl.ChangeType { case ... }
  • misleading comment → lineNumGutter comment now says "lipgloss style" instead of "raw ANSI", with note explaining why it's safe (gutter is concatenated before content, so the reset doesn't break outer backgrounds)
  • toggleLineNumbers focus gate → now checks m.focus == paneDiff before toggling, aligned with toggleWrapMode/toggleCollapsedMode
  • nestif lint → extracted wrap block into renderWrappedCollapsedLine() to reduce nesting complexity introduced by the line numbers guard

Copy link
Copy Markdown
Owner

@umputun umputun left a comment

Choose a reason for hiding this comment

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

lgtm, all review points addressed. thx

@umputun umputun merged commit e317b3c into umputun:master Apr 5, 2026
1 check passed
@rashpile rashpile deleted the line-numbers branch April 5, 2026 19:45
alexander-akhmetov added a commit to alexander-akhmetov/revdiff that referenced this pull request Apr 6, 2026
The line numbers toggle (L key) from umputun#21 was runtime-only.
This adds a CLI flag, env var, and config file option so users
can enable line numbers by default.
umputun pushed a commit that referenced this pull request Apr 6, 2026
The line numbers toggle (L key) from #21 was runtime-only.
This adds a CLI flag, env var, and config file option so users
can enable line numbers by default.
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.

Add line number gutter toggle (L key)

3 participants