feat(review): add go-to-line prompt - #808
Conversation
Add a go-to-line flow: `:` opens a line-number prompt in the status bar, and Enter jumps the current-line cursor to that line in the selected file so `c` can annotate it directly. Bare numbers use the current file's numbering (the note labels' `R` side); an `l` prefix targets the source file's numbering (the `L` side). The jump resolves through the shared revealLine path, and line-level navigation no longer depends on the cursor_line display option: with the marker off, the target cursor is synthesized on demand instead of falling back to the hunk. The command closes an open menu when triggered, reports failures as transient notices (which now outrank the filter summary), and the bundled vim-navigation example's command line moves from `:` to `;` since built-ins shadow extension chords by design.
|
@IAMLEIzZ is attempting to deploy a commit to the Modem Team on Vercel. A member of the Team first needs to authorize it. |
|
@IAMLEIzZ Any reason this is a draft? Should I take a look? |
Sorry for the delay — this week has been extremely busy and I haven't had a chance to review the latest version yet. I'll need to go through it properly before we proceed. I'll remove the draft status once it's ready for review. |
benvinegar
left a comment
benvinegar
left a comment
There was a problem hiding this comment.
Should we add : to the in-app help as well? It feels like a primary navigation shortcut.
This comment was generated by Pi using gpt-5.6-sol
| return "none"; | ||
| } | ||
|
|
||
| // An empty list means line-level navigation is hidden (`cursor_line = "off"`), not |
There was a problem hiding this comment.
An empty cursor list doesn’t always mean cursor_line is off—it can also happen with extension file views. In that case we may return "line" even though there are no bounds to scroll to.
Could we only synthesize the cursor when we know the raw diff is being rendered? Otherwise, we should fall back to the hunk.
This comment was generated by Pi using gpt-5.6-sol
| // An empty list means line-level navigation is hidden (`cursor_line = "off"`), not | ||
| // that the target row is missing: synthesize the line's cursor and reveal it exactly. | ||
| if (cursors.length === 0) { | ||
| revealLineCursor(lineCursorAt(cursors, fileId, hunkIndex, { side, line }), "reveal"); |
There was a problem hiding this comment.
This also breaks on zero-count sides. For example, :lN on a pure insertion can target an old-side row that doesn’t exist, but we still report a successful jump and allow a comment there.
Could we check that the target has a real rendered line first? A pure insertion/deletion test would be useful too.
This comment was generated by Pi using gpt-5.6-sol
2 participants
Closes #805
Summary
Adds a go-to-line flow to the review UI:
:opens a small line-number prompt in the status bar (the same inline-input pattern as the/file filter), and Enter jumps the current-line cursor to that line in the selected file soccan annotate it directly. A bare number uses the current file's numbering (theRside the note labels use); anlprefix such as:l42targets the source file's numbering (theLside). Escape cancels, and a line that is not part of the diff reports a transient notice naming the side instead of jumping silently.The jump resolves through the existing shared
revealLinepath that the sessionnavigatecommand uses, so scrolling, selection, and note targeting all land consistently. Line-level navigation is also decoupled from thecursor_linedisplay option: when the marker is off, the target cursor is synthesized on demand from the render plan (no full stop-list rebuild), so jumping and annotating stay precise instead of degrading to the hunk.Notes
hunk.review.gotoLineis registered in the shared command catalog with:as the default chord, is remappable via[keybindings], and closes an open menu when triggered.:to;, since built-in chords shadow extension chords by design.filter=…summary so they stay visible while a file filter is active.Testing
:2delivery, non-digit stripping, Escape cancel, new/old side targeting, closing an open menu,cursor_line = "off"jump and annotate precision, selected-file targeting, and the two-stage filter Escape.useTerminalReviewcontract tests updated for the synthesized-cursor reveal; vim example unit tests and PTY updated for the move to;.