GTST-19 refactor: improve coding style with pedantic lints and shared helpers

.github/workflows/ci.yml

@@ -20,7 +20,7 @@ jobs:
           components: clippy
       - uses: Swatinem/rust-cache@v2
       - run: cargo check --all-targets
-      - run: cargo clippy --all-targets -- -D warnings
+      - run: cargo clippy --all-targets
 
   test:
     name: Test

.gitignore

@@ -21,6 +21,9 @@ CLAUDE.md
 # Test
 test-folder/
 
+# Local docs
+gotchas.md
+
 # Local config
 .env
 *.local

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "gitsift"
-version = "0.1.1"
+version = "0.1.2"
 edition = "2024"
 description = "Git hunk sifter for code agents"
 license = "MIT"
@@ -21,3 +21,20 @@ similar     = { version = "2", features = ["text"] }
 assert_cmd = "2"
 predicates = "3"
 tempfile   = "3"
+
+[lints.rust]
+unsafe_code = "forbid"
+
+[lints.clippy]
+pedantic = { level = "warn", priority = -1 }
+# Re-allow noisy pedantic lints
+module_name_repetitions = "allow"
+missing_errors_doc      = "allow"
+must_use_candidate      = "allow"
+missing_panics_doc      = "allow"
+similar_names           = "allow"
+unnecessary_wraps       = "allow"
+# Enforce specific lints
+cloned_instead_of_copied          = "deny"
+redundant_closure_for_method_calls = "deny"
+unnested_or_patterns              = "deny"

clippy.toml

@@ -0,0 +1,3 @@
+too-many-arguments-threshold = 7
+too-many-lines-threshold = 250
+cognitive-complexity-threshold = 30

rust-toolchain.toml

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "stable"
+components = ["rustfmt", "clippy"]

rustfmt.toml

@@ -0,0 +1,3 @@
+edition = "2024"
+max_width = 100
+use_small_heuristics = "Max"

src/git/diff.rs

@@ -1,10 +1,11 @@
 use anyhow::{Context, Result};
-use git2::{Delta, DiffOptions, Repository};
+use git2::{Delta, Repository};
 use std::cell::RefCell;
 use std::collections::hash_map::DefaultHasher;
 use std::hash::{Hash, Hasher};
 use std::path::Path;
 
+use super::{delta_path, hunk_header, is_binary_delta};
 use crate::models::{DiffOutput, FileChange, FileStatus, Hunk, HunkLine, LineTag};
 
 /// Generate a stable hunk ID from file path, old start line, and header.
@@ -20,7 +21,7 @@ pub fn hunk_id(file_path: &str, old_start: u32, header: &str) -> String {
     format!("{:016x}", hasher.finish())
 }
 
-/// Map git2 Delta to our FileStatus.
+/// Map git2 Delta to our `FileStatus`.
 fn delta_to_status(delta: Delta) -> FileStatus {
     match delta {
         Delta::Added | Delta::Untracked => FileStatus::Added,
@@ -30,7 +31,7 @@ fn delta_to_status(delta: Delta) -> FileStatus {
     }
 }
 
-/// Mutable state shared across git2 foreach callbacks via RefCell.
+/// Mutable state shared across git2 foreach callbacks via `RefCell`.
 struct DiffState {
     files: Vec<FileChange>,
     current_file: Option<FileChange>,
@@ -39,11 +40,7 @@ struct DiffState {
 
 impl DiffState {
     fn new() -> Self {
-        Self {
-            files: Vec::new(),
-            current_file: None,
-            current_hunk: None,
-        }
+        Self { files: Vec::new(), current_file: None, current_hunk: None }
     }
 
     /// Flush current hunk into current file, then flush current file into files list.
@@ -76,18 +73,13 @@ impl DiffState {
 pub fn diff_unstaged(repo_path: &Path, file_filter: Option<&str>) -> Result<DiffOutput> {
     let repo = Repository::open(repo_path).context("failed to open git repository")?;
 
-    let mut opts = DiffOptions::new();
-    opts.context_lines(3);
-    opts.include_untracked(true);
-    opts.show_untracked_content(true);
-
+    let mut opts = super::diff_opts_with_untracked();
     if let Some(filter) = file_filter {
         opts.pathspec(filter);
     }
 
-    let diff = repo
-        .diff_index_to_workdir(None, Some(&mut opts))
-        .context("failed to generate diff")?;
+    let diff =
+        repo.diff_index_to_workdir(None, Some(&mut opts)).context("failed to generate diff")?;
 
     let state = RefCell::new(DiffState::new());
 
@@ -96,18 +88,13 @@ pub fn diff_unstaged(repo_path: &Path, file_filter: Option<&str>) -> Result<Diff
             let mut s = state.borrow_mut();
             s.flush_file();
 
-            let path = delta
-                .new_file()
-                .path()
-                .or_else(|| delta.old_file().path())
-                .map(|p| p.to_string_lossy().into_owned())
-                .unwrap_or_default();
-
             // Skip binary files
-            if delta.new_file().is_binary() || delta.old_file().is_binary() {
+            if is_binary_delta(&delta) {
                 return true;
             }
 
+            let path = delta_path(&delta);
+
             s.current_file = Some(FileChange {
                 path,
                 status: delta_to_status(delta.status()),
@@ -120,13 +107,9 @@ pub fn diff_unstaged(repo_path: &Path, file_filter: Option<&str>) -> Result<Diff
             let mut s = state.borrow_mut();
             s.flush_hunk();
 
-            let file_path = s
-                .current_file
-                .as_ref()
-                .map(|f| f.path.as_str())
-                .unwrap_or("");
+            let file_path = s.current_file.as_ref().map_or("", |f| f.path.as_str());
 
-            let header = String::from_utf8_lossy(hunk.header()).trim().to_string();
+            let header = hunk_header(&hunk);
 
             s.current_hunk = Some(Hunk {
                 id: hunk_id(file_path, hunk.old_start(), &header),
@@ -143,11 +126,7 @@ pub fn diff_unstaged(repo_path: &Path, file_filter: Option<&str>) -> Result<Diff
         Some(&mut |_delta, _hunk, line| {
             let mut s = state.borrow_mut();
 
-            let tag = match line.origin() {
-                '+' | '>' => LineTag::Insert,
-                '-' | '<' => LineTag::Delete,
-                _ => LineTag::Equal,
-            };
+            let tag = LineTag::from_origin(line.origin());
 
             let content = String::from_utf8_lossy(line.content()).into_owned();
 
@@ -193,8 +172,7 @@ mod tests {
             index.write().unwrap();
             let tree_oid = index.write_tree().unwrap();
             let tree = repo.find_tree(tree_oid).unwrap();
-            repo.commit(Some("HEAD"), &sig, &sig, "initial", &tree, &[])
-                .unwrap();
+            repo.commit(Some("HEAD"), &sig, &sig, "initial", &tree, &[]).unwrap();
         }
 
         (dir, repo)
@@ -208,19 +186,15 @@ mod tests {
         let tree_oid = index.write_tree().unwrap();
         let tree = repo.find_tree(tree_oid).unwrap();
         let head = repo.head().unwrap().peel_to_commit().unwrap();
-        repo.commit(Some("HEAD"), &sig, &sig, msg, &tree, &[&head])
-            .unwrap();
+        repo.commit(Some("HEAD"), &sig, &sig, msg, &tree, &[&head]).unwrap();
     }
 
     #[test]
     fn diff_modified_file() {
         let (dir, _repo) = setup_repo();
 
-        fs::write(
-            dir.path().join("hello.txt"),
-            "line 1\nline 2 modified\nline 3\nline 4\n",
-        )
-        .unwrap();
+        fs::write(dir.path().join("hello.txt"), "line 1\nline 2 modified\nline 3\nline 4\n")
+            .unwrap();
 
         let output = diff_unstaged(dir.path(), None).unwrap();
         assert_eq!(output.files.len(), 1);
@@ -252,10 +226,7 @@ mod tests {
         // Must have hunks with actual content (not empty)
         assert!(!new_file.hunks.is_empty(), "added file must have hunks");
         assert!(
-            new_file.hunks[0]
-                .lines
-                .iter()
-                .any(|l| l.tag == LineTag::Insert),
+            new_file.hunks[0].lines.iter().any(|l| l.tag == LineTag::Insert),
             "added file hunk must have insert lines"
         );
     }
@@ -335,11 +306,7 @@ mod tests {
 
         let output = diff_unstaged(dir.path(), None).unwrap();
         let big_file = output.files.iter().find(|f| f.path == "big.txt").unwrap();
-        assert!(
-            big_file.hunks.len() >= 2,
-            "expected 2+ hunks, got {}",
-            big_file.hunks.len()
-        );
+        assert!(big_file.hunks.len() >= 2, "expected 2+ hunks, got {}", big_file.hunks.len());
 
         // Each hunk should have a unique ID
         let ids: Vec<&str> = big_file.hunks.iter().map(|h| h.id.as_str()).collect();
@@ -361,11 +328,7 @@ mod tests {
     fn diff_line_numbers_correct() {
         let (dir, _repo) = setup_repo();
 
-        fs::write(
-            dir.path().join("hello.txt"),
-            "line 1\nINSERTED\nline 2\nline 3\n",
-        )
-        .unwrap();
+        fs::write(dir.path().join("hello.txt"), "line 1\nINSERTED\nline 2\nline 3\n").unwrap();
 
         let output = diff_unstaged(dir.path(), None).unwrap();
         let hunk = &output.files[0].hunks[0];
@@ -376,10 +339,7 @@ mod tests {
             .find(|l| l.tag == LineTag::Insert && l.content.contains("INSERTED"))
             .expect("should find inserted line");
 
-        assert!(
-            inserted.old_lineno.is_none(),
-            "insert has no old line number"
-        );
+        assert!(inserted.old_lineno.is_none(), "insert has no old line number");
         assert!(inserted.new_lineno.is_some(), "insert has new line number");
     }
 }

src/git/mod.rs

@@ -1,3 +1,48 @@
+use git2::DiffOptions;
+
 pub mod diff;
 pub mod stage;
 pub mod status;
+
+/// Default context lines for all diffs.
+const CONTEXT_LINES: u32 = 3;
+
+/// Create `DiffOptions` including untracked file content.
+///
+/// Used for: diff, status, and hunk metadata scanning.
+pub fn diff_opts_with_untracked() -> DiffOptions {
+    let mut opts = DiffOptions::new();
+    opts.context_lines(CONTEXT_LINES);
+    opts.include_untracked(true);
+    opts.show_untracked_content(true);
+    opts
+}
+
+/// Create `DiffOptions` for tracked files only.
+///
+/// Used for: applying hunks to the index (untracked files handled separately).
+pub fn diff_opts_tracked_only() -> DiffOptions {
+    let mut opts = DiffOptions::new();
+    opts.context_lines(CONTEXT_LINES);
+    opts
+}
+
+/// Extract the file path from a `DiffDelta`, preferring `new_file`.
+pub fn delta_path(delta: &git2::DiffDelta) -> String {
+    delta
+        .new_file()
+        .path()
+        .or_else(|| delta.old_file().path())
+        .map(|p| p.to_string_lossy().into_owned())
+        .unwrap_or_default()
+}
+
+/// Extract and clean the header string from a git2 `DiffHunk`.
+pub fn hunk_header(hunk: &git2::DiffHunk<'_>) -> String {
+    String::from_utf8_lossy(hunk.header()).trim().to_string()
+}
+
+/// Check if a `DiffDelta` represents a binary file.
+pub fn is_binary_delta(delta: &git2::DiffDelta) -> bool {
+    delta.new_file().is_binary() || delta.old_file().is_binary()
+}