Skip to content

feat: tab cycle through comments, comment auto-follow, cursor decoration-skip - #385

Merged
agavra merged 1 commit into
agavra:mainfrom
N4M3Z:feat-ux-polish
Jun 11, 2026
Merged

feat: tab cycle through comments, comment auto-follow, cursor decoration-skip#385
agavra merged 1 commit into
agavra:mainfrom
N4M3Z:feat-ux-polish

Conversation

@N4M3Z

@N4M3Z N4M3Z commented May 27, 2026

Copy link
Copy Markdown
Contributor

Reworked from scratch on top of current main — the wrap stuff is gone, this no
longer touches diff rendering or line counts.

What's left is the safe half of the original PR:

  • Tab cycle goes FileList → Diff → CommitSelector → Comments (reverse mirrors
    it), and tabbing into Comments/FileList unhides the left pane
  • j/k in the comment navigator scrolls the diff to the selected comment without
    stealing focus (Enter still jumps + focuses)
  • jump_to_file / page scrolls skip spacing + file-header rows so the cursor
    never lands on a non-actionable line — the skip logic is in free functions
    with unit tests
  • :help / :h, and :wrap as a short alias for :set wrap!

Word-boundary wrap will come back as its own PR, rebuilt against the current
wrap implementation and with rendering tests for the line-count math this time.

Tests: fmt/clippy clean, 972 passing. The 2 failures in vcs::git::cli::tests
are pre-existing — they run real git commit and inherit the host's
commit.gpgsign, so they time out against my YubiKey; same result on a clean
checkout of main.

@N4M3Z N4M3Z changed the title feat: word-boundary wrap, comment follow, and UX polish fix: UX polish (word-boundary wrap, comment follow, :commands and help) May 27, 2026
@N4M3Z N4M3Z mentioned this pull request May 27, 2026
6 tasks
@N4M3Z

N4M3Z commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

UX fixes, let me know if you want me to split it but most should be sane

@agavra

agavra commented May 27, 2026

Copy link
Copy Markdown
Owner

@N4M3Z I'm going to revert the old line wrapping PR in the meantime until I can battle test this a bit more. it looks like this introduces a few other cosmetic bugs like the file headers being broken
image

@agavra

agavra commented May 27, 2026

Copy link
Copy Markdown
Owner

I think I need to spend some time figuring out how to have better automated tests because anything that changes rendering line counts causes a lot of downstream problems. see this as well #384 (comment)

@N4M3Z

N4M3Z commented May 27, 2026

Copy link
Copy Markdown
Contributor Author

Yes, apologies, I'm testing by using so I discover these as I go. I'll contribute here if I find I need to fix more.

…ion-skip

Tab cycle reordered: FileList -> Diff -> CommitSelector -> Comments ->
FileList (reverse mirrors it). Tabbing to Comments or FileList reveals
the left pane.

Comment navigator auto-follows: j/k scrolls the diff to the selected
comment without stealing focus. Enter still jumps and focuses.

Cursor positioning (jump_to_file, scroll_down, scroll_up) skips
decoration lines (Spacing, FileHeader) so the cursor never lands on a
non-actionable line. Skip logic lives in free functions with unit
tests covering forward/backward walks and the max-line clamp.

`:help` / `:h` opens the help popup. `:wrap` is a short alias for
`:set wrap!`.
@N4M3Z N4M3Z changed the title fix: UX polish (word-boundary wrap, comment follow, :commands and help) feat: tab cycle through comments, comment auto-follow, cursor decoration-skip Jun 11, 2026
@N4M3Z

N4M3Z commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Reworked this on top of current main — dropped the wrap changes entirely, so
nothing here touches rendering or line counts anymore. Just the panel/cursor
UX bits plus :help/:wrap aliases.

I'll redo the word wrap as a separate PR against the current implementation,
with rendering tests for the line counts, before asking you to look again.

@agavra agavra left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @N4M3Z this LGTM. I like the comment cycle experience

@agavra
agavra merged commit 8f18cd9 into agavra:main Jun 11, 2026
4 checks passed
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.

2 participants