diff --git a/TODO.md b/TODO.md index d05e1bc..5945eb5 100644 --- a/TODO.md +++ b/TODO.md @@ -8,7 +8,7 @@ - [x] diff view: show the function/class context that a change hunk belongs to. - inline scope labels with sticky continuation, chosen before implementation - each side uses its own syntax tree; scope changes within a hunk get separate labels -- [ ] diff view: annotate folder with checkmark if all items under given folder has been checked AND the folder is collapsed. +- [x] diff view: annotate folder with checkmark if all items under given folder has been checked AND the folder is collapsed. - [ ] diff view: automatically detect changes and update the view when relevant bookmarks move change ids (regards bookmark-based diff view; i.e., user selected at least one bookmark in the before/after picker) - [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc. - [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding. diff --git a/docs/editing.md b/docs/editing.md index b140921..82dec4a 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -432,7 +432,9 @@ Use the ordinary **file-picker/file-explorer actions and bindings**: Space `o` o File rows show colored **A** (added, green), **M** (modified, yellow), **D** (removed, red), or **R** (renamed, cyan) markers in Tree and List views, including filtered results. Rename rows show `R old/path -> new/path`; directories have no file-status marker. -Mark or unmark a diff file with **`c` while browsing the changed-file tree**, **`:diff.review.toggle`**, or the command-palette action. With the changed-file explorer focused, it toggles the highlighted file without opening it; from a text pane, the command toggles the displayed file. Reviewed files show a green **`✓` immediately left of A/M/D/R, separated by one blank column**, in Tree and List views. The modification indicator keeps its existing indentation; the review marker uses the preceding gutter space instead of adding padding. Diff rows use the selection background and cursor rather than a `>` prefix. Directories and ordinary filesystem pickers cannot be marked. While filtering, including List view, `c` remains query text; use the palette or a control-key binding there. Explicit bindings take precedence over the default browsing shortcut. +Mark or unmark a diff file with **`c` while browsing the changed-file tree**, **`:diff.review.toggle`**, or the command-palette action. With the changed-file explorer focused, it toggles the highlighted file without opening it; from a text pane, the command toggles the displayed file. Reviewed files show a green **`✓` immediately left of A/M/D/R, separated by one blank column**, in Tree and List views. The modification indicator keeps its existing indentation; the review marker uses the preceding gutter space instead of adding padding. Diff rows use the selection background and cursor rather than a `>` prefix. Directory review marks cannot be toggled directly, and ordinary filesystem pickers have no review marks. While filtering, including List view, `c` remains query text; use the palette or a control-key binding there. Explicit bindings take precedence over the default browsing shortcut. + +Collapsed folders show a green **`✓`** when every changed file beneath them is reviewed, including nested files and descendants hidden by filtering. Expanded folders stay unmarked. Folder marks update automatically when a descendant's review mark or the changed-file catalog changes. Review marks belong only to the open comparison and survive filtering, file navigation, lazy loading, tabs, split/unified/orientation changes, and pinned checkout-only transitions. A new or recomputed comparison clears them. Editing a writable After file clears only that file's mark; unchanged refreshes and saves preserve it. Marks do not change document text, undo history, or repository state, and are not persisted after closing the comparison. diff --git a/src/files.rs b/src/files.rs index 54b59f9..b8b5e6b 100644 --- a/src/files.rs +++ b/src/files.rs @@ -370,6 +370,20 @@ impl FilePicker { }) } + /// Full catalog descendants, independent of filtering and folder expansion. + pub(crate) fn descendant_catalog_indices<'a>( + &'a self, + path: &'a Path, + ) -> impl Iterator + 'a { + let start = self + .nodes + .partition_point(|node| node.path.as_path() <= path); + self.nodes[start..] + .iter() + .take_while(move |node| node.path.starts_with(path)) + .filter_map(|node| node.catalog_index) + } + fn selected_node(&self) -> Option { self.rows.get(self.selected).map(|row| row.node) } diff --git a/src/tui.rs b/src/tui.rs index 28a117d..cc00e28 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -5692,10 +5692,22 @@ fn draw_file_list( .and_then(|file| file.old_path.as_deref()) .or(entry.old_path); let mut spans = Vec::with_capacity(8); - let reviewed = changed.is_some() - && entry.catalog_index.is_some_and(|index| { - changed_files.is_some_and(|(_, reviewed)| reviewed.contains(&index)) - }); + let reviewed = changed_files.is_some_and(|(_, reviewed)| { + if entry.is_dir { + if entry.expanded { + return false; + } + let mut descendants = + picker.descendant_catalog_indices(entry.path).peekable(); + descendants.peek().is_some() + && descendants.all(|index| reviewed.contains(&index)) + } else { + changed.is_some() + && entry + .catalog_index + .is_some_and(|index| reviewed.contains(&index)) + } + }); if changed_files.is_none() { spans.push(Span::raw(if selected { ">" } else { " " })); spans.push(Span::raw(" ")); @@ -9541,6 +9553,94 @@ mod tests { assert!(!screen_text(&terminal).contains('✓')); } + #[test] + fn collapsed_diff_folders_reflect_all_catalog_descendants() { + let files: Vec<_> = [ + "src/top.rs", + "src/deep/leaf.rs", + "src/deep/other.rs", + "srcish/unreviewed.rs", + "src", + "compact/a/b/only.rs", + ] + .into_iter() + .map(|path| { + let mut file = deletion_file(); + file.path = path.into(); + file + }) + .collect(); + let paths = || { + files + .iter() + .enumerate() + .map(|(index, file)| (index, file.path.clone())) + }; + let mut picker = FilePicker::from_paths(PathBuf::new(), paths()); + let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(100, 18)).unwrap(); + let mut check = |picker: &FilePicker, reviewed: &BTreeSet, label: &str, marked| { + terminal + .draw(|frame| { + draw_file_list( + frame, + picker, + frame.area(), + "", + &mut WheelPanels::default(), + Some((&files, reviewed)), + ); + }) + .unwrap(); + let text = screen_text(&terminal); + let row = text.lines().find(|row| row.contains(label)).unwrap(); + assert_eq!(row.contains('✓'), marked, "{row}"); + if marked { + let buffer = terminal.backend().buffer(); + let y = text.lines().position(|row| row.contains(label)).unwrap() as u16; + let x = (0..100).find(|&x| buffer[(x, y)].symbol() == "✓").unwrap(); + assert_eq!(buffer[(x, y)].fg, Color::Green); + } + }; + let mut reviewed = BTreeSet::from([0, 1]); + check(&picker, &reviewed, "src/", false); + reviewed.insert(2); + check(&picker, &reviewed, "src/", true); + check(&picker, &reviewed, "srcish/", false); + check(&picker, &reviewed, "compact/a/b/", false); + reviewed.insert(5); + check(&picker, &reviewed, "compact/a/b/", true); + + let selected = picker + .entries() + .position(|row| row.is_dir && row.path == std::path::Path::new("src")) + .unwrap(); + picker.selected = selected; + picker.right(); + check(&picker, &reviewed, "src/", false); + check(&picker, &reviewed, "deep/", true); + + reviewed.remove(&2); + picker.query = "leaf.rs".into(); + picker.refilter(); + let selected = picker + .entries() + .position(|row| row.is_dir && row.path == std::path::Path::new("src/deep")) + .unwrap(); + picker.selected = selected; + picker.accept(); + check(&picker, &reviewed, "deep/", false); + reviewed.insert(2); + check(&picker, &reviewed, "deep/", true); + reviewed.remove(&2); + check(&picker, &reviewed, "deep/", false); + + // A reverted file no longer in the picker must not block its folder's mark. + picker.replace_paths(paths().filter(|(index, _)| *index != 2)); + check(&picker, &reviewed, "deep/", true); + picker.accept(); + check(&picker, &reviewed, "deep/", false); + } + #[test] fn normal_file_markers_keep_clean_rows_and_match_diff_colors_in_every_view() { use crate::repository::WorkingFileChange;