diff --git a/README.md b/README.md index 1f809cc4..92ae7aa8 100644 --- a/README.md +++ b/README.md @@ -250,6 +250,7 @@ A first-session cheatsheet. Press `?` inside tuicr for the full reference. | `r` | Toggle file reviewed | | `R` | Toggle hunk reviewed | | `e` | Open focused file in `$EDITOR` | +| `Space` | Expand / collapse a generated file | | `y` | Copy review to clipboard | | `:edit` | Open focused file in `$EDITOR` | | `:submit` | Push review to GitHub or GitLab | diff --git a/docs/CONFIG.md b/docs/CONFIG.md index f30fce94..5ca6e1e4 100644 --- a/docs/CONFIG.md +++ b/docs/CONFIG.md @@ -58,6 +58,10 @@ pr_metadata = true comments_header = "## Local tuicr Comments" remote_comments_header = "## Existing GitHub Comments" legend = true + +[generated] +collapse = false +count = true ``` ## Options @@ -269,6 +273,45 @@ The example above produces an export that opens directly on the comment list: The top-level `export_legend` key predates this section and still works. When both are set, `legend` wins. When `[export]` omits `legend`, `export_legend` stays in force, so adding an `[export]` block to trim the intro will not switch the legend back on. +## Generated files + +Files marked as code-generated in `.gitattributes` can be collapsed so a review of hand-written code is not buried under regenerated protobufs, lockfiles, or API clients. + +A file counts as generated when `.gitattributes` sets either `linguist-generated` (GitHub's Linguist) or `gitlab-generated`. Both documented spellings work: + +```gitattributes +*.pb.go linguist-generated=true +schema.generated.ts gitlab-generated +vendor/** linguist-generated=true +``` + +Resolution is delegated to git itself, so nested `.gitattributes` files, `.git/info/attributes`, `core.attributesFile`, and `[attr]` macros all behave the way `git check-attr` does. An explicit opt-out — `-linguist-generated` or `linguist-generated=false` — wins over an opt-in on the other attribute, so a directory-wide rule can be excepted per file. + +```toml +[generated] +collapse = true +count = false +``` + +| Key | Default | Description | +| ---------- | ------- | --------------------------------------------------------------------------------------------------------------------------------- | +| `collapse` | `false` | Hide the diff body of generated files, leaving the header row. `Space` expands one file; `:set generated!` toggles all of them. | +| `count` | `true` | Count generated files toward the `Files · reviewed/total` progress indicator. When `false`, the file list also reports `N generated`. | + +`count` is deliberately independent of `collapse`: expanding a generated file to look at it should not add it to the denominator, or review progress would drop as a side effect of looking. You can also exclude generated files from progress without collapsing them. + +Runtime commands: + +| Command | Effect | +| ------------------- | ------------------------------------------------- | +| `:set generated` | Collapse generated files. | +| `:set nogenerated` | Show generated files. | +| `:set generated!`, `:generated` | Toggle. | + +`.gitattributes` is read from the local worktree, so this needs a git repository: mercurial and non-colocated jujutsu repos report nothing as generated, and in pull request mode the attributes come from your local checkout rather than the pull request's head. Attributes are re-read on reload (`:e`), so editing `.gitattributes` mid-session takes effect without a restart. + +To hide files from a review entirely rather than collapse them, use `.tuicrignore` below. + ## .tuicrignore tuicr reads `.tuicrignore` from the repository root and excludes matching files from all review diffs. Rules follow gitignore-style pattern matching, including `!` negation. diff --git a/docs/KEYBINDINGS.md b/docs/KEYBINDINGS.md index cf628d91..8365d008 100644 --- a/docs/KEYBINDINGS.md +++ b/docs/KEYBINDINGS.md @@ -76,6 +76,7 @@ Shown below the file tree when local comments or visible remote PR threads exist |-----|--------| | `r` | Toggle file reviewed | | `R` | Toggle hunk reviewed | +| `Space` | Expand / collapse the file under the cursor, independent of reviewed/generated status (diff panel) | | `c` | Add line comment (or file comment if not on a diff line) | | `C` | Add file comment | | `c` | Add review comment | @@ -143,6 +144,9 @@ In command mode, | `:set commits` | Show inline commit selector | | `:set nocommits` | Hide inline commit selector | | `:set commits!` | Toggle inline commit selector | +| `:set generated` | Collapse files marked generated in `.gitattributes` | +| `:set nogenerated` | Show generated files | +| `:set generated!`, `:generated` | Toggle generated file collapse | | `:clear` | Clear all comments | | `:clearc` | Clear comments without clearing reviewed marks | | `:version` | Show tuicr version | diff --git a/src/app/annotations.rs b/src/app/annotations.rs index 3d3029d6..5d336237 100644 --- a/src/app/annotations.rs +++ b/src/app/annotations.rs @@ -60,6 +60,13 @@ impl App { if self.file_line_count_cache.is_empty() { self.populate_file_line_count_cache(); } + // Which files are code-generated has to be known before the walk + // below decides whose body to emit. Detection is hooked here rather + // than at each of the fifteen `self.diff_files = ...` assignments + // because every one of them rebuilds annotations afterwards, and a + // missed call site would desynchronize the annotations from the + // render. Repeat calls over an unchanged file set do no work. + self.detect_generated_files(); self.line_annotations.clear(); @@ -163,10 +170,10 @@ impl App { .push(AnnotatedLine::FileHeader { file_idx }); } - // If reviewed, skip all content for this file. Single-file - // view ignores the reviewed-collapse since the user - // explicitly focused this file. - if self.session.is_file_reviewed(path) && !self.is_single_file_view { + // If collapsed, skip all content for this file. Single-file + // view ignores the collapse since the user explicitly focused + // this file. + if self.is_file_collapsed(file) && !self.is_single_file_view { continue; } diff --git a/src/app/diff_load.rs b/src/app/diff_load.rs index 52066e03..0d32fecd 100644 --- a/src/app/diff_load.rs +++ b/src/app/diff_load.rs @@ -581,6 +581,14 @@ impl App { self.diff_files = diff_files; self.clear_expanded_gaps(); + // Reload is the point where an edited `.gitattributes` should take + // effect, so discard the probe memo and re-detect eagerly — the height + // math below runs before `rebuild_annotations` would have done it. + // `collapse_override` survives: it is the user's override, not + // detected state. + self.invalidate_generated_detection(); + self.detect_generated_files(); + self.sort_files_by_directory(false); self.populate_file_line_count_cache(); self.expand_all_dirs(); diff --git a/src/app/generated.rs b/src/app/generated.rs new file mode 100644 index 00000000..422670ac --- /dev/null +++ b/src/app/generated.rs @@ -0,0 +1,161 @@ +use super::*; + +impl App { + /// Apply resolved `[generated]` settings and detect matching files. + /// + /// Called from startup after config load, because `App::build` runs before + /// the config is applied and would otherwise detect against the defaults. + pub fn apply_generated_config(&mut self, config: &GeneratedConfig) { + self.collapse_generated = config.collapse(); + self.count_generated = config.count(); + // Annotations were built during `App::build`, before detection could + // run, so any newly collapsed file is not reflected in them yet. + if self.detect_generated_files() { + self.rebuild_annotations(); + } + } + + /// Whether `.gitattributes` has to be consulted at all. + /// + /// Both features off means the detected set stays empty, which is what + /// makes the default configuration free: no repository is opened, and the + /// render path's guards short-circuit before any lookup. + fn generated_detection_enabled(&self) -> bool { + self.collapse_generated || !self.count_generated + } + + /// Repository root to resolve `.gitattributes` against, if there is one. + /// + /// PR mode's `root_path` is the synthetic `forge:host/owner/repo` identity + /// rather than a directory, so it falls back to the local checkout the + /// forge backend resolved — which is only set when the checkout matches + /// the PR's target repository, so a foreign checkout can't mis-mark files. + fn generated_attributes_root(&self) -> Option { + if self.vcs_info.root_path.is_absolute() { + return Some(self.vcs_info.root_path.clone()); + } + self.forge_backend + .as_deref() + .and_then(|backend| backend.local_checkout_path()) + } + + /// Look up `.gitattributes` for every diff file not probed yet, returning + /// whether that grew the generated set. + /// + /// Probed paths are remembered, so a repeat call over an unchanged file + /// set costs one hash lookup per file and no libgit2 work. That memo is + /// also the staleness test, which is what lets `rebuild_annotations` keep + /// detection current without each of the fifteen `diff_files` assignments + /// having to remember to trigger it. + pub(in crate::app) fn detect_generated_files(&mut self) -> bool { + if !self.generated_detection_enabled() { + return false; + } + if self + .diff_files + .iter() + .all(|file| self.generated_probed.contains(file.display_path())) + { + return false; + } + let Some(root) = self.generated_attributes_root() else { + return false; + }; + + let unprobed: Vec = self + .diff_files + .iter() + .map(|file| file.display_path()) + .filter(|path| !self.generated_probed.contains(*path)) + .cloned() + .collect(); + + let detected = crate::profile::time("generated.detect", || { + crate::generated::detect_generated(&root, &unprobed) + }); + + self.generated_probed.extend(unprobed); + let changed = !detected.is_empty(); + self.generated_files.extend(detected); + changed + } + + /// Forget which paths were probed so the next detection re-reads + /// `.gitattributes`. Called on explicit reload, so editing the attributes + /// file takes effect without restarting tuicr. + pub(in crate::app) fn invalidate_generated_detection(&mut self) { + self.generated_probed.clear(); + self.generated_files.clear(); + } + + /// Whether to surface this path as code-generated. + /// + /// Gated on the same condition as detection rather than on the detected + /// set being non-empty, so the decoration is a function of the current + /// settings and not of what the session has detected in the past. The set + /// is deliberately *not* discarded when the feature is switched off — that + /// is what makes re-enabling free — so keying the decoration off it would + /// make `:set generated` / `:set nogenerated` asymmetric: the labels, the + /// dimmed tree rows, and the counter would all survive a toggle back off + /// and never return to how the session started. + #[inline] + pub fn is_generated_file(&self, path: &Path) -> bool { + self.generated_detection_enabled() && self.generated_files.contains(path) + } + + /// Number of diff files surfaced as code-generated. + pub fn generated_file_count(&self) -> usize { + if !self.generated_detection_enabled() { + return 0; + } + self.diff_files + .iter() + .filter(|file| self.generated_files.contains(file.display_path())) + .count() + } + + /// `(reviewed, total)` file counts for the review progress indicator. + /// + /// Generated files drop out of both halves when `[generated] count` is + /// off — out of the numerator too, or marking one reviewed would push the + /// count past its own total. + pub fn review_progress(&self) -> (usize, usize) { + if self.count_generated || self.generated_files.is_empty() { + return (self.reviewed_count(), self.file_count()); + } + let mut reviewed = 0; + let mut total = 0; + for file in &self.diff_files { + let path = file.display_path(); + if self.generated_files.contains(path) { + continue; + } + total += 1; + if self.session.is_file_reviewed(path) { + reviewed += 1; + } + } + (reviewed, total) + } + + /// Set the runtime collapse toggle, running detection if it was skipped + /// while both features were off. + pub fn set_collapse_generated(&mut self, collapse: bool) { + self.collapse_generated = collapse; + self.detect_generated_files(); + self.rebuild_annotations(); + + let count = self.generated_file_count(); + if !collapse { + self.set_message("Showing generated files"); + } else if count == 0 { + self.set_warning("No files marked generated in .gitattributes"); + } else { + self.set_message(format!("Collapsing {count} generated file(s)")); + } + } + + pub fn toggle_collapse_generated(&mut self) { + self.set_collapse_generated(!self.collapse_generated); + } +} diff --git a/src/app/init.rs b/src/app/init.rs index c6ccd979..60afb78e 100644 --- a/src/app/init.rs +++ b/src/app/init.rs @@ -559,6 +559,11 @@ impl App { saved_inline_selection: None, path_filter: path_filter.map(|s| s.to_string()), export: ExportConfig::default(), + collapse_generated: false, + count_generated: true, + generated_files: HashSet::new(), + collapse_override: HashMap::new(), + generated_probed: HashSet::new(), }; // Auto-hide file list when path filter matches exactly one file if app.path_filter.is_some() && app.diff_files.len() == 1 { diff --git a/src/app/mod.rs b/src/app/mod.rs index c088c57e..0e304b56 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -6,7 +6,7 @@ use chrono::Utc; use ratatui::style::Color; use crate::comment_vim::CommentVimEditor; -use crate::config::{CommentTypeConfig, ExportConfig}; +use crate::config::{CommentTypeConfig, ExportConfig, GeneratedConfig}; use crate::editor::EditorTarget; use crate::error::{Result, TuicrError}; use crate::forge::context::{ContextProvider, ForgeContextProvider, VcsContextProvider}; @@ -1289,6 +1289,36 @@ pub struct App { pub path_filter: Option, /// Resolved `[export]` settings shaping the generated review markdown. pub export: ExportConfig, + /// Whether files `.gitattributes` marks as code-generated have their diff + /// body hidden. Seeded from `[generated] collapse`, flipped at runtime by + /// `:set generated!`. Off by default, so tuicr ships behaving as before. + pub collapse_generated: bool, + /// Whether generated files count toward review progress. Seeded from + /// `[generated] count`, which defaults to true. Independent of + /// `collapse_generated`: expanding a generated file to peek at it must + /// not add it to the denominator. + pub count_generated: bool, + /// Diff files that `.gitattributes` marks as code-generated, keyed by + /// display path. + /// + /// Path-keyed rather than index-keyed because reloads, watch ticks, and + /// commit-selection changes all replace `diff_files` wholesale and shift + /// every index. Empty whenever detection is disabled, which is what lets + /// the render path short-circuit for free. + pub generated_files: HashSet, + /// Per-file collapse override set with `Space`, taking precedence over + /// the reviewed/generated defaults `is_file_collapsed` would otherwise + /// derive. Keyed by path rather than index so it survives the reloads + /// and commit-selection changes that replace `diff_files` wholesale. + pub collapse_override: HashMap, + /// Display paths already looked up in `.gitattributes`. + /// + /// Doubles as the staleness test for detection: a diff file set with no + /// unprobed path needs no work, which is how `rebuild_annotations` can + /// keep detection current without every `diff_files` assignment having to + /// remember to trigger it. Cleared on explicit reload so an edited + /// `.gitattributes` takes effect. + pub generated_probed: HashSet, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1503,6 +1533,7 @@ mod comments; mod commits; mod diff_load; mod gaps; +mod generated; mod init; mod modes; mod navigation; diff --git a/src/app/navigation.rs b/src/app/navigation.rs index 74ff89f4..d3576e3d 100644 --- a/src/app/navigation.rs +++ b/src/app/navigation.rs @@ -841,16 +841,15 @@ impl App { continue; } let path = file.display_path(); - let is_reviewed = self.session.is_file_reviewed(path); + // Mutually exclusive, so only one file-state lookup per file. if !single { cumulative += 1; // File header - } - if !single && is_reviewed { - // multi-file collapsed: no body, no trailing spacing - continue; - } - if single && is_reviewed { + if self.is_file_collapsed(file) { + // multi-file collapsed: no body, no trailing spacing + continue; + } + } else if self.session.is_file_reviewed(path) { cumulative += 1; // banner } if let Some(review) = self.session.files.get(path) { @@ -994,7 +993,7 @@ impl App { } pub(in crate::app) fn file_render_height(&self, file_idx: usize, file: &DiffFile) -> usize { - if self.session.is_file_reviewed(file.display_path()) { + if self.is_file_collapsed(file) { return 1; // collapsed: header only } 1 + self.file_render_body_height(file_idx, file) // header + body diff --git a/src/app/reviewed.rs b/src/app/reviewed.rs index 0ed7133a..46bb9940 100644 --- a/src/app/reviewed.rs +++ b/src/app/reviewed.rs @@ -273,6 +273,66 @@ impl App { } } + /// Whether a file's diff body is hidden, leaving only its header row. + /// + /// Checked in override order. `Space` sets an explicit per-file entry in + /// `collapse_override` that always wins, because it is what lets a + /// reviewed or generated file be peeked at (or re-hidden) without + /// touching either status. Failing that, reviewed files collapse so a + /// long review stream shrinks as it is worked through. Failing that, + /// files `.gitattributes` marks as code-generated collapse when the user + /// opted in. The annotation builder, both diff renderers, and the + /// scroll-height math all have to agree on this or the cursor lands on + /// rows that aren't drawn, so they share this one predicate. + /// + /// Deliberately says nothing about single-file view: the call sites + /// disagree about it (`file_render_height` ignores it, the renderers + /// honor it), so each keeps its own gate rather than having one folded + /// in here. + /// + /// Both renderers and `hunk_positions` call this once per file per frame, + /// so it has to stay cheap. The checks are ordered so the default + /// configuration still costs exactly one map lookup: `collapse_override` + /// is empty unless the user has pressed `Space`, and `collapse_generated` + /// is false unless the user opted in, so a bare bool test short-circuits + /// the rest. + #[inline] + pub fn is_file_collapsed(&self, file: &DiffFile) -> bool { + let path = file.display_path(); + if let Some(&collapsed) = self.collapse_override.get(path) { + return collapsed; + } + if self.session.is_file_reviewed(path) { + return true; + } + self.collapse_generated && self.generated_files.contains(path) + } + + /// Toggle whether the file under the cursor shows its diff body, + /// independent of its reviewed or generated status — those only decide + /// the default before `Space` is pressed. + /// + /// Keyed by path rather than index so the override survives the reloads + /// and commit-selection changes that replace `diff_files` wholesale. + pub fn toggle_file_collapse(&mut self) -> bool { + let Some(file) = self.current_file() else { + return false; + }; + let path = file.display_path().clone(); + let currently_collapsed = self.is_file_collapsed(file); + self.collapse_override.insert(path, !currently_collapsed); + self.rebuild_annotations(); + let file_idx = self.diff_state.current_file_idx; + self.diff_state.cursor_line = self.calculate_file_scroll_offset(file_idx); + self.ensure_cursor_visible(); + self.set_message(if currently_collapsed { + "File expanded" + } else { + "File collapsed" + }); + true + } + pub fn file_count(&self) -> usize { self.diff_files.len() } diff --git a/src/app/tests/generated_tests.rs b/src/app/tests/generated_tests.rs new file mode 100644 index 00000000..111a427e --- /dev/null +++ b/src/app/tests/generated_tests.rs @@ -0,0 +1,421 @@ +use std::fs; +use std::path::{Path, PathBuf}; + +use tempfile::{TempDir, tempdir}; + +use crate::app::*; +use crate::config::GeneratedConfig; +use crate::model::{DiffFile, DiffHunk, DiffLine, FileStatus, LineOrigin}; +use crate::vcs::traits::{VcsBackend, VcsInfo, VcsType}; + +struct StubVcs(VcsInfo); +impl VcsBackend for StubVcs { + fn info(&self) -> &VcsInfo { + &self.0 + } + fn get_working_tree_diff( + &self, + _hl: &crate::syntax::SyntaxHighlighter, + ) -> crate::error::Result> { + Ok(Vec::new()) + } + fn fetch_context_lines( + &self, + _path: &Path, + _status: FileStatus, + _ref_commit: Option<&str>, + _start: u32, + _end: u32, + ) -> crate::error::Result> { + Ok(Vec::new()) + } + fn file_line_count( + &self, + _path: &Path, + _status: FileStatus, + _ref_commit: Option<&str>, + ) -> crate::error::Result { + Ok(0) + } +} + +fn hunk() -> DiffHunk { + let lines = (1..=3) + .map(|i| DiffLine { + origin: LineOrigin::Context, + content: format!("line {i}"), + old_lineno: Some(i), + new_lineno: Some(i), + highlighted_spans: None, + }) + .collect(); + DiffHunk { + header: "@@ -1,3 +1,3 @@".to_string(), + lines, + old_start: 1, + old_count: 3, + new_start: 1, + new_count: 3, + } +} + +fn file(path: &str) -> DiffFile { + let hunks = vec![hunk()]; + let content_hash = DiffFile::compute_content_hash(&hunks); + DiffFile { + old_path: None, + new_path: Some(PathBuf::from(path)), + status: FileStatus::Modified, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + } +} + +/// A git repository whose root `.gitattributes` marks `api.pb.go` generated. +fn repo() -> TempDir { + let dir = tempdir().expect("tempdir"); + git2::Repository::init(dir.path()).expect("init repo"); + fs::write( + dir.path().join(".gitattributes"), + "*.pb.go linguist-generated=true\n", + ) + .expect("write .gitattributes"); + dir +} + +fn app_in(dir: &TempDir, paths: &[&str]) -> App { + let vcs_info = VcsInfo { + root_path: dir.path().to_path_buf(), + head_commit: "head".into(), + branch_name: Some("main".into()), + vcs_type: VcsType::Git, + }; + let session = ReviewSession::new( + vcs_info.root_path.clone(), + vcs_info.head_commit.clone(), + vcs_info.branch_name.clone(), + SessionDiffSource::WorkingTree, + ); + App::build( + Box::new(StubVcs(vcs_info.clone())), + vcs_info, + crate::theme::Theme::dark(), + None, + false, + paths.iter().copied().map(file).collect(), + session, + DiffSource::WorkingTree, + InputMode::Normal, + Vec::new(), + None, + None, + ) + .expect("build app") +} + +fn collapsing_app(dir: &TempDir, paths: &[&str]) -> App { + let mut app = app_in(dir, paths); + app.apply_generated_config(&GeneratedConfig { + collapse: Some(true), + count: None, + }); + app +} + +fn file_named(app: &App, path: &str) -> DiffFile { + app.diff_files + .iter() + .find(|f| f.display_path() == Path::new(path)) + .expect("file in diff set") + .clone() +} + +#[test] +fn default_config_never_reads_gitattributes() { + // Both features default off, so detection must not run at all — not even + // to open the repository. An empty probe set is the observable proof. + let dir = repo(); + let app = app_in(&dir, &["api.pb.go", "src/main.rs"]); + + assert!(app.generated_probed.is_empty()); + assert!(app.generated_files.is_empty()); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn collapse_hides_generated_files_only() { + let dir = repo(); + let app = collapsing_app(&dir, &["api.pb.go", "src/main.rs"]); + + assert!(app.is_generated_file(Path::new("api.pb.go"))); + assert!(!app.is_generated_file(Path::new("src/main.rs"))); + assert!(app.is_file_collapsed(&file_named(&app, "api.pb.go"))); + assert!(!app.is_file_collapsed(&file_named(&app, "src/main.rs"))); +} + +#[test] +fn space_expansion_overrides_collapse() { + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + app.diff_state.current_file_idx = 0; + + assert!(app.toggle_file_collapse()); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); + + assert!(app.toggle_file_collapse()); + assert!(app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn space_manually_collapses_a_file_that_is_not_generated() { + // Space is not scoped to generated files: it toggles any file's body, + // manually collapsing one that defaulted to expanded. + let dir = repo(); + let mut app = collapsing_app(&dir, &["src/main.rs"]); + app.diff_state.current_file_idx = 0; + + assert!(!app.is_file_collapsed(&file_named(&app, "src/main.rs"))); + assert!(app.toggle_file_collapse()); + assert!(app.is_file_collapsed(&file_named(&app, "src/main.rs"))); +} + +#[test] +fn turning_collapse_off_stops_collapsing_already_detected_files() { + // The detected set is not cleared when the feature is switched off, so + // the predicate has to gate on the runtime flag rather than on the set + // being empty. + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + assert!(app.is_file_collapsed(&file_named(&app, "api.pb.go"))); + + app.toggle_collapse_generated(); + + assert!(!app.generated_files.is_empty()); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn toggling_collapse_off_returns_to_the_starting_appearance() { + // The detected set is kept when the feature is switched off so that + // re-enabling is free, which means the decoration must not be keyed off + // it: `:generated` twice from a default start has to land back exactly + // where it began, labels and counter included. + let dir = repo(); + let mut app = app_in(&dir, &["api.pb.go", "src/main.rs"]); + + let initial = ( + app.is_generated_file(Path::new("api.pb.go")), + app.generated_file_count(), + app.review_progress(), + app.is_file_collapsed(&file_named(&app, "api.pb.go")), + ); + assert_eq!(initial, (false, 0, (0, 2), false)); + + app.toggle_collapse_generated(); + assert_eq!( + ( + app.is_generated_file(Path::new("api.pb.go")), + app.generated_file_count(), + app.review_progress(), + app.is_file_collapsed(&file_named(&app, "api.pb.go")), + ), + (true, 1, (0, 2), true) + ); + + app.toggle_collapse_generated(); + assert_eq!( + ( + app.is_generated_file(Path::new("api.pb.go")), + app.generated_file_count(), + app.review_progress(), + app.is_file_collapsed(&file_named(&app, "api.pb.go")), + ), + initial, + "toggling collapse off must restore the starting appearance" + ); + // The set itself is retained, so re-enabling costs no libgit2 work. + assert!(!app.generated_files.is_empty()); +} + +#[test] +fn count_exclusion_alone_still_labels_without_being_collapsed() { + // `count = false, collapse = false` is a legitimate configuration: the + // file is surfaced and dropped from progress, but nothing is hidden by + // default. + let dir = repo(); + let mut app = app_in(&dir, &["api.pb.go", "src/main.rs"]); + app.apply_generated_config(&GeneratedConfig { + collapse: None, + count: Some(false), + }); + app.diff_state.current_file_idx = app + .diff_files + .iter() + .position(|f| f.display_path() == Path::new("api.pb.go")) + .expect("generated file"); + + assert!(app.is_generated_file(Path::new("api.pb.go"))); + assert_eq!(app.generated_file_count(), 1); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn expansion_survives_a_diff_file_rebuild_that_shifts_indices() { + // Reloads, watch ticks, and commit-selection changes all replace + // `diff_files` wholesale. State keyed by index would follow the wrong + // file afterwards. + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + app.diff_state.current_file_idx = 0; + app.toggle_file_collapse(); + + app.diff_files = vec![file("src/main.rs"), file("api.pb.go")]; + app.rebuild_annotations(); + + assert_eq!( + app.diff_files[1].display_path(), + &PathBuf::from("api.pb.go") + ); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn space_expands_a_reviewed_file_without_un_reviewing_it() { + // The whole point of a per-file override: peeking at a reviewed file's + // diff must not touch its reviewed status, unlike the old behavior where + // `r` was the only way to see it again. + let dir = repo(); + let mut app = app_in(&dir, &["src/main.rs"]); + app.diff_state.current_file_idx = 0; + app.toggle_reviewed(); + assert!(app.is_file_collapsed(&file_named(&app, "src/main.rs"))); + + assert!(app.toggle_file_collapse()); + assert!(!app.is_file_collapsed(&file_named(&app, "src/main.rs"))); + assert!(app.session.is_file_reviewed(&PathBuf::from("src/main.rs"))); + + assert!(app.toggle_file_collapse()); + assert!(app.is_file_collapsed(&file_named(&app, "src/main.rs"))); + assert!(app.session.is_file_reviewed(&PathBuf::from("src/main.rs"))); +} + +#[test] +fn detection_runs_for_count_exclusion_without_collapse() { + // `count` is deliberately not gated by `collapse`: excluding generated + // files from review progress is useful on its own. + let dir = repo(); + let mut app = app_in(&dir, &["api.pb.go", "src/main.rs"]); + app.apply_generated_config(&GeneratedConfig { + collapse: None, + count: Some(false), + }); + + assert_eq!(app.generated_file_count(), 1); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); + assert_eq!(app.review_progress(), (0, 1)); +} + +#[test] +fn progress_counts_generated_files_by_default() { + let dir = repo(); + let app = collapsing_app(&dir, &["api.pb.go", "src/main.rs"]); + + assert_eq!(app.generated_file_count(), 1); + assert_eq!(app.review_progress(), (0, 2)); +} + +#[test] +fn reviewing_an_excluded_generated_file_cannot_overrun_the_total() { + // Both halves of the fraction have to drop the file, or marking it + // reviewed would report 2/1. + let dir = repo(); + let mut app = app_in(&dir, &["api.pb.go", "src/main.rs"]); + app.apply_generated_config(&GeneratedConfig { + collapse: None, + count: Some(false), + }); + + let generated_idx = app + .diff_files + .iter() + .position(|f| f.display_path() == Path::new("api.pb.go")) + .expect("generated file"); + app.toggle_reviewed_for_file_idx(generated_idx, false); + + assert_eq!(app.review_progress(), (0, 1)); +} + +#[test] +fn invalidation_re_reads_edited_gitattributes() { + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + assert!(app.is_file_collapsed(&file_named(&app, "api.pb.go"))); + + fs::write( + dir.path().join(".gitattributes"), + "*.pb.go -linguist-generated\n", + ) + .expect("rewrite .gitattributes"); + app.invalidate_generated_detection(); + app.rebuild_annotations(); + + assert!(!app.is_generated_file(Path::new("api.pb.go"))); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn set_generated_commands_drive_the_runtime_toggle() { + let dir = repo(); + let mut app = app_in(&dir, &["api.pb.go"]); + + for (command, expected) in [ + ("set generated", true), + ("set nogenerated", false), + ("set generated!", true), + ("generated", false), + ] { + app.enter_command_mode(); + app.command_buffer = command.to_string(); + crate::handler::handle_command_action(&mut app, crate::input::Action::SubmitInput); + + assert_eq!( + app.collapse_generated, expected, + "`:{command}` should leave collapse={expected}" + ); + assert_eq!(app.input_mode, InputMode::Normal); + } + // The commands have to work from the `collapse = false` default, or the + // opt-in default would be intolerable: the very first `:set generated` + // must trigger detection that startup skipped. + assert!(app.generated_files.contains(Path::new("api.pb.go"))); +} + +#[test] +fn space_in_the_diff_panel_expands_the_generated_file_under_the_cursor() { + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + app.focused_panel = FocusedPanel::Diff; + app.diff_state.current_file_idx = 0; + + crate::handler::handle_diff_action(&mut app, crate::input::Action::ToggleExpand); + + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} + +#[test] +fn detection_is_skipped_when_there_is_no_local_repository_root() { + // PR mode's root is the synthetic `forge:host/owner/repo` identity, and + // without a matching local checkout there is nothing to read attributes + // from. It must degrade to "nothing is generated", not panic. + let dir = repo(); + let mut app = collapsing_app(&dir, &["api.pb.go"]); + app.vcs_info.root_path = PathBuf::from("forge:github.com/agavra/tuicr"); + app.invalidate_generated_detection(); + app.rebuild_annotations(); + + assert!(app.generated_files.is_empty()); + assert!(!app.is_file_collapsed(&file_named(&app, "api.pb.go"))); +} diff --git a/src/app/tests/mod.rs b/src/app/tests/mod.rs index cb97dafc..d6455301 100644 --- a/src/app/tests/mod.rs +++ b/src/app/tests/mod.rs @@ -5,6 +5,7 @@ mod decoration_skip_tests; mod diff_source_tests; mod expand_gap_tests; mod find_source_line_tests; +mod generated_tests; mod persistence_merge_tests; mod pr_info_tests; mod scroll_behavior_tests; diff --git a/src/app/tests/single_file_view_tests.rs b/src/app/tests/single_file_view_tests.rs index da7fb8d0..cfc7b86b 100644 --- a/src/app/tests/single_file_view_tests.rs +++ b/src/app/tests/single_file_view_tests.rs @@ -516,6 +516,42 @@ fn release_then_press_walks_when_keyboard_enhancement_supported() { assert!(!app.down_released_since_arm); } +#[test] +fn is_file_collapsed_tracks_reviewed_state() { + let files = vec![ + file("a.rs", vec![hunk(1, 3)]), + file("b.rs", vec![hunk(1, 3)]), + ]; + let mut app = app_with(files); + + let a = app.diff_files[0].clone(); + let b = app.diff_files[1].clone(); + assert!(!app.is_file_collapsed(&a)); + assert!(!app.is_file_collapsed(&b)); + + app.toggle_reviewed_for_file_idx(0, false); + assert!(app.is_file_collapsed(&a)); + assert!(!app.is_file_collapsed(&b)); +} + +#[test] +fn is_file_collapsed_ignores_single_file_view() { + // The predicate deliberately says nothing about single-file view: its + // consumers disagree about it (`file_render_height` ignores it, the + // renderers honor it), so each keeps its own gate. Locking that here + // stops a future change from folding the check in and silently + // altering `file_render_height`. + let files = vec![file("a.rs", vec![hunk(1, 3)])]; + let mut app = app_with(files); + app.toggle_reviewed_for_file_idx(0, false); + let a = app.diff_files[0].clone(); + + app.is_single_file_view = false; + assert!(app.is_file_collapsed(&a)); + app.is_single_file_view = true; + assert!(app.is_file_collapsed(&a)); +} + #[test] fn effective_file_height_is_zero_for_non_current_in_single_file_view() { let files = vec![ diff --git a/src/config/mod.rs b/src/config/mod.rs index bb49fca9..40763546 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -94,6 +94,36 @@ impl ExportConfig { } } +/// `[generated]` section settings for files that `.gitattributes` marks as +/// code-generated (`linguist-generated` or `gitlab-generated`). +/// +/// Both fields are optional so the shipped defaults stay distinguishable from +/// an explicit setting, matching `[export]`. The defaults reproduce tuicr's +/// behavior before generated-file support existed, so an absent section is +/// exactly today's behavior — and skips attribute detection entirely. +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(default)] +pub struct GeneratedConfig { + /// Whether to hide the diff body of generated files, leaving the header. + /// `Space` still expands an individual file. + pub collapse: Option, + /// Whether generated files count toward the reviewed/total progress + /// indicator. Deliberately independent of `collapse`: expanding a + /// generated file to peek at it must not add it to the denominator, or + /// review progress would regress as a side effect of looking. + pub count: Option, +} + +impl GeneratedConfig { + pub fn collapse(&self) -> bool { + self.collapse.unwrap_or(false) + } + + pub fn count(&self) -> bool { + self.count.unwrap_or(true) + } +} + #[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] #[serde(default)] pub struct AppConfig { @@ -146,6 +176,9 @@ pub struct AppConfig { /// `[export]` section settings. `None` means "no override"; downstream /// code should treat it as `ExportConfig::default()`. pub export: Option, + /// `[generated]` section settings. `None` means "no override"; downstream + /// code should treat it as `GeneratedConfig::default()`. + pub generated: Option, } impl AppConfig { @@ -193,6 +226,7 @@ const KNOWN_KEYS: &[&str] = &[ "username", "forge", "export", + "generated", ]; const FORGE_KNOWN_KEYS: &[&str] = &["comment_type_prefix"]; @@ -206,6 +240,8 @@ const EXPORT_KNOWN_KEYS: &[&str] = &[ "legend", ]; +const GENERATED_KNOWN_KEYS: &[&str] = &["collapse", "count"]; + #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct ConfigLoadOutcome { pub config: Option, @@ -431,6 +467,9 @@ fn load_config_from_path(path: &Path) -> Result { export: table .get("export") .and_then(|v| parse_export(v, &mut warnings)), + generated: table + .get("generated") + .and_then(|v| parse_generated(v, &mut warnings)), }; for key in table.keys() { @@ -512,6 +551,36 @@ fn parse_export(value: &Value, warnings: &mut Vec) -> Option) -> Option { + let Some(table) = value.as_table() else { + warnings + .push("Warning: Config key 'generated' must be a table; ignoring value".to_string()); + return None; + }; + + for key in table.keys() { + if !GENERATED_KNOWN_KEYS.contains(&key.as_str()) { + warnings.push(format!( + "Warning: Unknown config key 'generated.{key}', ignoring" + )); + } + } + + let cfg = GeneratedConfig { + collapse: read_section_bool(table, "generated", "collapse", warnings), + count: read_section_bool(table, "generated", "count", warnings), + }; + + if cfg == GeneratedConfig::default() { + None + } else { + Some(cfg) + } +} + /// Like `read_bool`, but emits a `
.` qualified warning so the /// user can locate the misconfigured field. fn read_section_bool( @@ -1651,6 +1720,137 @@ scope_line = "no" ); } + // generated + + #[test] + fn generated_accessors_reproduce_pre_feature_behavior() { + // Both defaults are load-bearing: they are what makes an absent + // `[generated]` section behave exactly as tuicr did before generated + // files were recognized, and skip attribute detection entirely. + let cfg = GeneratedConfig::default(); + assert!(!cfg.collapse()); + assert!(cfg.count()); + } + + #[test] + fn should_default_generated_to_none_when_section_missing() { + let outcome = parse_config(""); + assert_eq!( + outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()), + None + ); + assert!(outcome.warnings.is_empty()); + } + + #[test] + fn should_default_generated_to_none_when_section_is_empty_table() { + let outcome = parse_config("[generated]\n"); + assert_eq!( + outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()), + None + ); + assert!(outcome.warnings.is_empty()); + } + + #[test] + fn should_parse_generated_section_overriding_defaults() { + let outcome = parse_config( + r#"[generated] +collapse = true +count = false +"#, + ); + let generated = outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()) + .expect("generated section should parse"); + assert!(generated.collapse()); + assert!(!generated.count()); + assert!(outcome.warnings.is_empty()); + } + + #[test] + fn should_leave_unset_generated_keys_as_none() { + let outcome = parse_config( + r#"[generated] +collapse = true +"#, + ); + let generated = outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()) + .expect("generated section should parse"); + assert_eq!(generated.count, None); + assert!(generated.count()); + } + + #[test] + fn should_warn_on_unknown_generated_keys() { + let outcome = parse_config( + r#"[generated] +collapse = true +patterns = ["*.pb.go"] +"#, + ); + let generated = outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()) + .expect("generated section should parse"); + assert!(generated.collapse()); + assert_eq!( + outcome.warnings, + vec!["Warning: Unknown config key 'generated.patterns', ignoring".to_string()] + ); + } + + #[test] + fn should_warn_and_ignore_generated_bool_with_invalid_type() { + let outcome = parse_config( + r#"[generated] +collapse = "yes" +"#, + ); + assert_eq!( + outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()), + None + ); + assert_eq!( + outcome.warnings, + vec![ + "Warning: Config key 'generated.collapse' must be a boolean; ignoring value" + .to_string() + ] + ); + } + + #[test] + fn should_warn_when_generated_is_not_a_table() { + let outcome = parse_config("generated = true\n"); + assert_eq!( + outcome + .config + .as_ref() + .and_then(|cfg| cfg.generated.clone()), + None + ); + assert_eq!( + outcome.warnings, + vec!["Warning: Config key 'generated' must be a table; ignoring value".to_string()] + ); + } + // resolved export precedence #[test] diff --git a/src/generated.rs b/src/generated.rs new file mode 100644 index 00000000..aa84fc61 --- /dev/null +++ b/src/generated.rs @@ -0,0 +1,170 @@ +use std::collections::HashSet; +use std::path::{Path, PathBuf}; + +use git2::{AttrCheckFlags, AttrValue, Repository}; + +/// Attributes that mark a file as code-generated. GitHub's Linguist and +/// GitLab each define their own name; a file marked by either counts. +const GENERATED_ATTRS: [&str; 2] = ["linguist-generated", "gitlab-generated"]; + +/// Which of `paths` the repository's gitattributes mark as code-generated. +/// +/// Delegating to libgit2 rather than parsing `.gitattributes` ourselves means +/// nested `.gitattributes`, `.git/info/attributes`, `core.attributesFile`, and +/// `[attr]` macros all resolve the way git resolves them, for free. +/// +/// Returns an empty set — never an error — when there is no git repository to +/// ask: mercurial, non-colocated jujutsu, and pull requests without a local +/// checkout all land here, and none of them should block a review. +pub fn detect_generated(repo_root: &Path, paths: &[PathBuf]) -> HashSet { + let mut generated = HashSet::new(); + if paths.is_empty() { + return generated; + } + let Ok(repo) = Repository::discover(repo_root) else { + return generated; + }; + for path in paths { + if is_generated(&repo, path) { + generated.insert(path.clone()); + } + } + generated +} + +/// Whether either generated attribute is affirmatively set for `path`. +/// +/// An explicit opt-out on *either* attribute beats an opt-in on the other, and +/// a value we don't recognize counts as "not generated". Both rules fail toward +/// showing the diff: a needlessly shown diff is an annoyance, a needlessly +/// hidden one is unreviewed code. +fn is_generated(repo: &Repository, path: &Path) -> bool { + let mut opted_in = false; + for name in GENERATED_ATTRS { + let Ok(raw) = repo.get_attr(path, name, AttrCheckFlags::FILE_THEN_INDEX) else { + continue; + }; + // `get_attr` returns a sentinel string for a set-but-valueless + // attribute, so the raw value has to be interpreted through + // `AttrValue` rather than compared directly. Getting this wrong would + // read GitLab's bare `gitlab-generated` and its `-gitlab-generated` + // opt-out as the same thing. + match AttrValue::from_string(raw) { + // `attr` (GitLab's documented form) or `attr=true` (GitHub's). + AttrValue::True | AttrValue::String("true") => opted_in = true, + // `-attr` or `attr=false`. + AttrValue::False | AttrValue::String("false") => return false, + // `!attr`, absent, or a value neither forge documents. + _ => {} + } + } + opted_in +} + +#[cfg(test)] +mod tests { + use std::fs; + + use tempfile::{TempDir, tempdir}; + + use super::*; + + /// A git repository with the given `.gitattributes` content at its root. + fn repo_with_attributes(contents: &str) -> TempDir { + let dir = tempdir().expect("failed to create temp dir"); + Repository::init(dir.path()).expect("failed to init repo"); + fs::write(dir.path().join(".gitattributes"), contents) + .expect("failed to write .gitattributes"); + dir + } + + fn detect(dir: &TempDir, paths: &[&str]) -> HashSet { + let paths: Vec = paths.iter().map(PathBuf::from).collect(); + detect_generated(dir.path(), &paths) + } + + #[test] + fn detects_github_and_gitlab_attribute_forms() { + // `attr=true` yields `AttrValue::String("true")` while bare `attr` + // yields `AttrValue::True`. Both mean generated, and a naive + // `is_some()` check on the raw value would conflate them with the + // opt-out forms below. + let dir = repo_with_attributes( + "github.pb.go linguist-generated=true\ngitlab.pb.go gitlab-generated\n", + ); + + let generated = detect(&dir, &["github.pb.go", "gitlab.pb.go", "src/main.rs"]); + + assert_eq!( + generated, + HashSet::from([PathBuf::from("github.pb.go"), PathBuf::from("gitlab.pb.go"),]) + ); + } + + #[test] + fn treats_explicit_opt_out_forms_as_not_generated() { + let dir = repo_with_attributes( + "dash.pb.go -linguist-generated\nvalue.pb.go linguist-generated=false\nbang.pb.go !linguist-generated\n", + ); + + let generated = detect(&dir, &["dash.pb.go", "value.pb.go", "bang.pb.go"]); + + assert!(generated.is_empty(), "unexpected: {generated:?}"); + } + + #[test] + fn ignores_values_neither_forge_documents() { + let dir = repo_with_attributes("weird.pb.go linguist-generated=maybe\n"); + + assert!(detect(&dir, &["weird.pb.go"]).is_empty()); + } + + #[test] + fn opt_out_on_either_attribute_beats_opt_in_on_the_other() { + let dir = repo_with_attributes( + "a.pb.go linguist-generated=true -gitlab-generated\nb.pb.go -linguist-generated gitlab-generated\n", + ); + + let generated = detect(&dir, &["a.pb.go", "b.pb.go"]); + + assert!(generated.is_empty(), "unexpected: {generated:?}"); + } + + #[test] + fn resolves_nested_gitattributes() { + // Proves libgit2 is doing the attribute resolution rather than us + // reading only the root file. + let dir = repo_with_attributes(""); + let nested = dir.path().join("proto"); + fs::create_dir(&nested).expect("failed to create nested dir"); + fs::write( + nested.join(".gitattributes"), + "*.pb.go linguist-generated=true\n", + ) + .expect("failed to write nested .gitattributes"); + + let generated = detect(&dir, &["proto/api.pb.go", "root.pb.go"]); + + assert_eq!(generated, HashSet::from([PathBuf::from("proto/api.pb.go")])); + } + + #[test] + fn returns_empty_set_outside_a_git_repository() { + let dir = tempdir().expect("failed to create temp dir"); + fs::write( + dir.path().join(".gitattributes"), + "a.pb.go linguist-generated\n", + ) + .expect("failed to write .gitattributes"); + let paths = vec![PathBuf::from("a.pb.go")]; + + assert!(detect_generated(dir.path(), &paths).is_empty()); + } + + #[test] + fn returns_empty_set_without_opening_a_repository_for_no_paths() { + let dir = repo_with_attributes("*.pb.go linguist-generated=true\n"); + + assert!(detect_generated(dir.path(), &[]).is_empty()); + } +} diff --git a/src/handler.rs b/src/handler.rs index 2cf015e5..fde0b100 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -45,6 +45,15 @@ const COMMAND_SPECS: &[CommandSpec] = &[ CommandSpec::new(&["set commits"], CommandKind::SetCommitsVisible(true)), CommandSpec::new(&["set nocommits"], CommandKind::SetCommitsVisible(false)), CommandSpec::new(&["set commits!"], CommandKind::ToggleCommits), + CommandSpec::new(&["set generated"], CommandKind::SetGeneratedCollapsed(true)), + CommandSpec::new( + &["set nogenerated"], + CommandKind::SetGeneratedCollapsed(false), + ), + CommandSpec::new( + &["generated", "set generated!"], + CommandKind::ToggleGenerated, + ), CommandSpec::new(&["diff"], CommandKind::Diff), CommandSpec::new(&["focus", "f"], CommandKind::Focus), CommandSpec::new(&["stage"], CommandKind::Stage), @@ -117,6 +126,8 @@ enum CommandKind { SetVim(bool), SetCommitsVisible(bool), ToggleCommits, + SetGeneratedCollapsed(bool), + ToggleGenerated, Diff, Focus, Stage, @@ -810,6 +821,14 @@ fn dispatch_command(app: &mut App, kind: CommandKind) -> CommandAfterDispatch { app.toggle_commit_selector(); CommandAfterDispatch::ExitCommandMode } + CommandKind::SetGeneratedCollapsed(collapse) => { + app.set_collapse_generated(collapse); + CommandAfterDispatch::ExitCommandMode + } + CommandKind::ToggleGenerated => { + app.toggle_collapse_generated(); + CommandAfterDispatch::ExitCommandMode + } CommandKind::Diff => { app.toggle_diff_view_mode(); CommandAfterDispatch::ExitCommandMode @@ -1398,6 +1417,16 @@ pub fn handle_diff_action(app: &mut App, action: Action) { } } } + // Space already means "expand what's under the cursor" in the file + // list and the commit selector; in the diff panel it was unbound, so + // it takes on the same meaning here: toggle whether the current + // file's diff body is shown, regardless of why it defaulted to + // collapsed or expanded. + Action::ToggleExpand => { + if !app.toggle_file_collapse() { + handle_shared_normal_action(app, action); + } + } Action::SelectFileFull => { if let Some(hit) = app.get_gap_at_cursor() { match hit { diff --git a/src/lib.rs b/src/lib.rs index 94483a9b..1fdcf4b3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod config; pub mod editor; pub mod error; pub mod forge; +pub mod generated; pub mod handler; pub mod hash; pub mod input; diff --git a/src/main.rs b/src/main.rs index 2190b723..d001cf70 100644 --- a/src/main.rs +++ b/src/main.rs @@ -303,6 +303,7 @@ fn main() -> anyhow::Result<()> { app.toggle_single_file_view(); } app.export = cfg.resolved_export(); + app.apply_generated_config(&cfg.generated.clone().unwrap_or_default()); if cfg.cursor_line == Some(false) { app.cursor_line_highlight = false; } diff --git a/src/ui/diff_side_by_side.rs b/src/ui/diff_side_by_side.rs index a3fc6384..a18844b2 100644 --- a/src/ui/diff_side_by_side.rs +++ b/src/ui/diff_side_by_side.rs @@ -404,8 +404,9 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R continue; } let path = file.display_path(); - let is_reviewed = app.session.is_file_reviewed(path); + // Mutually exclusive branches, as in the unified renderer: each + // side pays for only the file-state lookup it actually needs. if !app.is_single_file_view { let indicator = cursor_indicator_spaced(line_idx, ctx.current_line_idx); let header_text = crate::ui::diff_view::file_header_prefix_text(app, file); @@ -418,15 +419,14 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R ), ])); line_idx += 1; - } - // If file is reviewed (and we're in multi-file view), skip the - // body. Single-file view keeps the focused file visible under a - // dimmed banner. - if is_reviewed && !app.is_single_file_view { - continue; - } - if is_reviewed && app.is_single_file_view { + // Collapsed files render as the header alone. + if app.is_file_collapsed(file) { + continue; + } + } else if app.session.is_file_reviewed(path) { + // Single-file view keeps the focused file visible under a + // dimmed banner. let indicator = cursor_indicator(line_idx, ctx.current_line_idx); lines.push(Line::from(vec![ Span::styled(indicator, styles::current_line_indicator_style(&app.theme)), diff --git a/src/ui/diff_unified.rs b/src/ui/diff_unified.rs index ea8b05e5..e1573706 100644 --- a/src/ui/diff_unified.rs +++ b/src/ui/diff_unified.rs @@ -246,12 +246,14 @@ pub(super) fn render_unified_diff(frame: &mut Frame, app: &mut App, area: Rect) continue; } let path = file.display_path(); - let is_reviewed = app.session.is_file_reviewed(path); - // The `═══ filename ═══` separator is redundant in single-file - // view: the status bar and file list already name the file, and - // the wide bar of `═` characters confuses horizontal scrolling. + // Multi-file and single-file view are mutually exclusive here, so + // this is one branch rather than two guarded conditions: each side + // then pays for only the file-state lookup it actually needs. if !app.is_single_file_view { + // The `═══ filename ═══` separator is redundant in single-file + // view: the status bar and file list already name the file, and + // the wide bar of `═` characters confuses horizontal scrolling. let indicator = cursor_indicator_spaced(line_idx, current_line_idx); let header_text = crate::ui::diff_view::file_header_prefix_text(app, file); lines.push(Line::from(vec![ @@ -263,15 +265,14 @@ pub(super) fn render_unified_diff(frame: &mut Frame, app: &mut App, area: Rect) ), ])); line_idx += 1; - } - // If file is reviewed (and we're in multi-file view), skip - // rendering the body. In single-file view the user explicitly - // focused this file, so show its content under a dimmed banner. - if is_reviewed && !app.is_single_file_view { - continue; - } - if is_reviewed && app.is_single_file_view { + // Collapsed files render as the header alone. + if app.is_file_collapsed(file) { + continue; + } + } else if app.session.is_file_reviewed(path) { + // Single-file view shows a reviewed file's content anyway -- + // the user explicitly focused it -- under a dimmed banner. let indicator = cursor_indicator(line_idx, current_line_idx); lines.push(Line::from(vec![ Span::styled(indicator, styles::current_line_indicator_style(&app.theme)), diff --git a/src/ui/diff_view.rs b/src/ui/diff_view.rs index 15ef48dd..b429137f 100644 --- a/src/ui/diff_view.rs +++ b/src/ui/diff_view.rs @@ -30,14 +30,22 @@ pub(super) fn file_header_prefix_text(app: &App, file: &DiffFile) -> String { let path = file.display_path(); let is_reviewed = app.session.is_file_reviewed(path); let review_mark = if is_reviewed { "✓ " } else { "" }; + // Shown whether or not the body is collapsed: once a generated file is + // expanded the label is the only thing left saying why it was hidden. + let generated_mark = if app.is_generated_file(path) { + " [generated]" + } else { + "" + }; if file.is_commit_message || app.is_pristine_mode { - format!("═══ {}{} ", review_mark, path.display()) + format!("═══ {}{}{} ", review_mark, path.display(), generated_mark) } else { format!( - "═══ {}{} [{}] ", + "═══ {}{} [{}]{} ", review_mark, path.display(), - file.status.as_char() + file.status.as_char(), + generated_mark ) } } diff --git a/src/ui/file_list.rs b/src/ui/file_list.rs index 6a605ed6..2b1003b4 100644 --- a/src/ui/file_list.rs +++ b/src/ui/file_list.rs @@ -19,11 +19,13 @@ const UNREVIEWED_BOX: &str = "\u{25a2}"; // ▢ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { let focused = app.focused_panel == FocusedPanel::FileList; - let title = format!( - " Files \u{00b7} {}/{} ", - app.reviewed_count(), - app.file_count() - ); + let (reviewed, total) = app.review_progress(); + let generated = app.generated_file_count(); + let title = if generated == 0 { + format!(" Files \u{00b7} {reviewed}/{total} ") + } else { + format!(" Files \u{00b7} {reviewed}/{total} \u{00b7} {generated} generated ") + }; let block = Block::default() .title(title) .borders(Borders::ALL) @@ -148,7 +150,19 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { styles::file_status_style(&app.theme, status), )); } - spans.push(Span::raw(filename.to_string())); + // Generated files read as dimmed so the tree shows + // what is skippable at a glance. Deliberately not a + // glyph: `▶` already means "collapsed directory" in + // this panel, and the ▣/▢ checkbox stays about + // reviewed state alone. + if app.is_generated_file(path) { + spans.push(Span::styled( + filename.to_string(), + styles::dim_style(&app.theme), + )); + } else { + spans.push(Span::raw(filename.to_string())); + } Line::from(spans) } } diff --git a/src/ui/help_popup.rs b/src/ui/help_popup.rs index 93c6f77c..addb0e0b 100644 --- a/src/ui/help_popup.rs +++ b/src/ui/help_popup.rs @@ -395,6 +395,13 @@ pub fn render_help(frame: &mut Frame, app: &mut App) { ), Span::raw("Toggle hunk reviewed"), ]), + Line::from(vec![ + Span::styled( + " Space ", + Style::default().add_modifier(Modifier::BOLD), + ), + Span::raw("Expand / collapse the current file"), + ]), Line::from(vec![ Span::styled( " c ", @@ -746,6 +753,27 @@ pub fn render_help(frame: &mut Frame, app: &mut App) { ), Span::raw(" Toggle inline commit selector"), ]), + Line::from(vec![ + Span::styled( + " :set generated", + Style::default().add_modifier(Modifier::BOLD), + ), + Span::raw(" Collapse files marked generated in .gitattributes"), + ]), + Line::from(vec![ + Span::styled( + " :set nogenerated", + Style::default().add_modifier(Modifier::BOLD), + ), + Span::raw(" Show generated files"), + ]), + Line::from(vec![ + Span::styled( + " :set generated!", + Style::default().add_modifier(Modifier::BOLD), + ), + Span::raw(" Toggle generated file collapse"), + ]), Line::from(vec![ Span::styled( " :clear ",