Skip to content

Commit aa73dfa

Browse files
committed
fix: invalidate width caches on same-length list mutation
Length-keyed width caches missed same-length content swaps (e.g. a file rename or commit-summary edit), leaving the right-scroll bound clamped to the old longest entry. Route every list mutation through set_files/set_commits/set_commit_files helpers that clear the cache on every assignment.
1 parent 8f76852 commit aa73dfa

7 files changed

Lines changed: 66 additions & 9 deletions

File tree

src/app.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,38 @@ mod tests {
751751
);
752752
}
753753

754+
#[test]
755+
fn snapshot_invalidates_path_width_cache_on_same_length_rename() {
756+
let (snapshot, tx) = dummy_snapshot_channel();
757+
let mut app = App {
758+
snapshot,
759+
..app_with_files(vec!["short.rs"])
760+
};
761+
// Prime the width cache by reading the right-scroll bound once.
762+
app.file_scroll_right();
763+
// Rename to a longer path while keeping the file count constant.
764+
// Length-keyed invalidation alone would miss this; the cache must
765+
// clear on every `set_files` assignment.
766+
tx.send(SnapshotMsg::Ok(
767+
RepoSnapshot {
768+
files: vec![ChangedFile::new(
769+
"a_much_longer_renamed_path.rs".to_string(),
770+
ChangeStatus::Modified,
771+
)],
772+
tracking: None,
773+
},
774+
HashMap::new(),
775+
))
776+
.unwrap();
777+
app.poll_snapshot();
778+
// Drive enough right-scrolls to reach the new max; if the cache were
779+
// stale we would clamp at the old (shorter) bound.
780+
for _ in 0..20 {
781+
app.file_scroll_right();
782+
}
783+
assert!(app.status_view.file_scroll_x >= "short.rs".chars().count());
784+
}
785+
754786
#[test]
755787
fn snapshot_refresh_with_no_filter_matches_clears_stale_diff() {
756788
let (snapshot, tx) = dummy_snapshot_channel();

src/app/focus.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ impl App {
1111
self.log_view.commit_scroll_x = 0;
1212
match self.with_repo(|repo| load_commit_log(repo, COMMIT_LOG_LIMIT)) {
1313
Ok(commits) => {
14-
self.log_view.commits = commits;
14+
self.log_view.set_commits(commits);
1515
self.log_view.selected = 0;
1616
self.load_commit_diff_for_selected();
1717
}
1818
Err(e) => {
1919
tracing::warn!(error = %e, "failed to load commit log");
20-
self.log_view.commits.clear();
20+
self.log_view.set_commits(Vec::new());
2121
self.log_view.selected = 0;
2222
self.status = Some(format!("git error: {e}"));
2323
}

src/app/navigation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ impl App {
313313
};
314314
match self.with_repo(|repo| load_commit_files(repo, oid)) {
315315
Ok(files) => {
316-
self.log_view.commit_files = files;
316+
self.log_view.set_commit_files(files);
317317
self.log_view.file_selected = 0;
318318
self.log_view.drill_down = true;
319319
if self.log_view.commit_files.is_empty() {

src/app/session_io.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ impl App {
8888
return;
8989
}
9090
};
91-
self.log_view.commits = commits;
91+
self.log_view.set_commits(commits);
9292
self.log_view.selected = state
9393
.log_selected
9494
.min(self.log_view.commits.len().saturating_sub(1));
@@ -112,7 +112,7 @@ impl App {
112112
};
113113
match self.with_repo(|repo| load_commit_files(repo, oid)) {
114114
Ok(files) => {
115-
self.log_view.commit_files = files;
115+
self.log_view.set_commit_files(files);
116116
self.log_view.drill_down = true;
117117
if self.log_view.commit_files.is_empty() {
118118
self.log_view.file_selected = 0;

src/app/snapshot_io.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ impl App {
2626
.files
2727
.get(self.status_view.selected)
2828
.map(|f| f.path.clone());
29-
self.status_view.files = snapshot.files;
29+
self.status_view.set_files(snapshot.files);
3030
self.status_view.recompute_filter();
3131
self.tracking = snapshot.tracking;
3232
self.merge_hot_table(mtimes);

src/ui/log_view.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,28 @@ pub struct LogView {
2020
}
2121

2222
impl LogView {
23+
/// Replace `commits` and invalidate the summary-width cache. See
24+
/// `StatusView::set_files` for the same-length rationale.
25+
pub(crate) fn set_commits(&mut self, commits: Vec<CommitEntry>) {
26+
self.commits = commits;
27+
self.commit_width_cache.set(None);
28+
}
29+
30+
/// Replace `commit_files` and invalidate the file-width cache so a
31+
/// same-length drill-in into a different commit doesn't reuse the
32+
/// previous commit's max path width.
33+
pub(crate) fn set_commit_files(&mut self, files: Vec<ChangedFile>) {
34+
self.commit_files = files;
35+
self.commit_files_width_cache.set(None);
36+
}
37+
2338
/// Exit drill-down so the upper pane shows the commit list again. Clears
2439
/// the file list and resets file-side cursors/scroll so a later drill-in
2540
/// starts from a clean state.
2641
pub fn reset_drill_down(&mut self) {
2742
self.drill_down = false;
2843
self.commit_files.clear();
44+
self.commit_files_width_cache.set(None);
2945
self.file_selected = 0;
3046
self.file_scroll_x = 0;
3147
}

src/ui/status_view.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,22 @@ pub struct StatusView {
2323
pub hot_table: HashMap<String, SystemTime>,
2424
/// Memoized longest-path char width, keyed by `files.len()`. Used by
2525
/// `upper_scroll_x_max` so the right-arrow keystroke does not walk every
26-
/// path on every press. Invalidated on length change; in this app the
27-
/// snapshot worker replaces `files` wholesale every tick so length-keyed
28-
/// invalidation is reliable enough for scroll bounds.
26+
/// path on every press. Length-keyed invalidation alone misses
27+
/// same-length content swaps (e.g. renames), so mutations must go through
28+
/// `set_files`, which clears this cell on every assignment.
2929
pub(crate) path_width_cache: Cell<Option<(usize, usize)>>,
3030
}
3131

3232
impl StatusView {
33+
/// Replace `files` and invalidate the path-width cache so a same-length
34+
/// snapshot whose contents changed (e.g. a file rename) does not leave a
35+
/// stale right-scroll bound. Length-keyed invalidation alone misses the
36+
/// rename case; clearing the cell on every assignment closes that hole.
37+
pub(crate) fn set_files(&mut self, files: Vec<ChangedFile>) {
38+
self.files = files;
39+
self.path_width_cache.set(None);
40+
}
41+
3342
/// Clear the search query and its lowercase cache together so callers
3443
/// can't accidentally reset only one and leave the cache stale.
3544
pub fn clear_search(&mut self) {

0 commit comments

Comments
 (0)