feat: add diff_watch_interval + fix some bugs - #566
Conversation
|
I didn't add it to this PR, but I think there are also some ways we can improve rendering and refreshing of really large diffs. (200+ files). This watcher works pretty well with ~100-ish file diffs right now. Files appear to be the limiting perf factor more than size of diff. |
agavra
left a comment
There was a problem hiding this comment.
Thanks @arjansingh this is cool. When you said that it works for "new commits" what do you mean by that?
Otherwise just one small nit and I think we need to rebase/resolve a minor merge conflict and we're good to go.
| /// Maximum visible completion candidates in the command prompt popup. | ||
| const COMMAND_COMPLETION_MAX_ROWS: usize = 7; | ||
|
|
||
| const DIFF_WATCH_GLYPH: &str = "\u{25c9}"; // ◉ |
There was a problem hiding this comment.
I don't think we should add this to the footer, it's not particularly helpful to permanently have hat there
There was a problem hiding this comment.
sounds good. i can remove.
I meant that when I commit changes, the commit list updates with the "new commit" I just added. |
d294da5 to
1c2f876
Compare
Reviewing uncommitted changes while an agent edits files in the background meant pressing `:e` to see anything new. Setting `diff_watch_interval_ms` to a millisecond value re-reads the diff on that interval instead. It defaults to disabled, and `0` disables it too, so an absent key changes nothing. Applying a reload clears expanded context, re-expands collapsed folders and moves the cursor, so a fingerprint over each file's path, status, flags and content hash decides whether a tick has anything worth applying. That check runs against a parse that resolves no grammars, which fingerprints the same as a highlighted one and drops the diff read on an idle tick from 395ms to 27ms on a 200 file branch. The real fetch runs on a worker thread, so a tick that does find a change never blocks the thread reading keystrokes. Resolving the fetch source before fetching also fixes a standing bug where a reload widened a narrowed commit selection back out. The same tick rebuilds the inline commit pane, so a commit written mid-review appears and the staged and unstaged rows follow the tree; staging a file leaves the combined diff byte-identical, so nothing the fingerprint can see reports it. Rebuilding renumbers the pane, so the cached per-commit diffs are dropped with it, the way every other path that replaces the pane already does.
Scrolling down a long diff with `j` or the arrow key stopped short of the end, leaving the rest unreachable. Moving the cursor down worked out the correct scroll position, accounting for lines that wrap across several rows, then capped the result to one line per keypress. A wrapped line needs a larger jump, so the cap discarded part of the answer and the shortfall built up. Once the cursor reached the last line the capping block stopped running at all, and the view froze there. Remove the cap. While the viewport and the scroll margin hold steady, one line of cursor movement never needs more than one line of scroll, so the cap did nothing outside the wrapped case it broke. Moving the cursor up never had the cap and never had this bug, so both directions now behave the same way.
…a commit All three look their file up in the session by path, and only registered files are there. Narrowing the inline commit pane loaded a fresh diff without registering it, so a commit-only file could be neither marked nor commented on. The two review marks return silently; commenting fails with `session does not contain file`. Staged and unstaged files kept working, because the startup load registered them much earlier. The three ways of loading that diff each wrote out the same install block, which is why all three missed the same call. They now share one install, with the registration in it. Hunk marks are preserved rather than pruned, matching the pull-request narrow path: a narrowed selection is a partial view of a wider review, and hunks it hides are still reviewed in that wider scope.
1c2f876 to
bef66ee
Compare
|
Thanks @arjansingh - LGTM! |
Description
diff_watch_interval_ms: You can set an interval inconfig.tomlfortuicrto watch for changes. Each tick runs a cheap check first, and only spawns a thread to fetch the real diff when something actually changed. That keeps an idle ticks fast. It's off by default and0also disables it, so nothing changes unless you opt in. It disabled for PR reviews and--all-files. I confirmed this works for new commits, staged changes, and unstaged changes.fix nav scrolling: I noticed that when I
jtoward the bottom of a long diff, sometimes it would stop short and leave the last lines unreachable. It only happens when lines wrap. I fixed that. Of note,Ghas the same symptom through a different function,jump_to_bottom, which I have not touched in the interest of finishing Feature Request - diff_watch_interval_ms for local diff changes #488. Should be a similar fix.fix review marking in a commit: I noticed that pressing
rorRon a file that only exists in a commit did nothing at all. Commenting on that same file fails too, withsession does not contain file. The PR side already registers these files correctly, so the local side was just inconsistent. Not good if you or your agent are reviewing a huge set of commits and want to mark something off or point out something that happened in the past. I fixed that too.Tests added for features and fixes of course. They are actually the majority of this diff.
Closes #488.