fix: fix the bugs from the code review

README.org

@@ -16,6 +16,13 @@ To get the recent changes comparing to the default branch you can type:
 $ git-changed
 #+end_src
 
+In a subdirectory, the tool prints the paths relative to that directory, like ~git status~. The glob also matches the relative path:
+
+#+begin_src bash
+$ cd src
+$ git-changed '*.rs' | xargs rustfmt
+#+end_src
+
 *** Default branch
 
 Per default the script uses the ~init.defaultBranch~ if that is configured, and falls back to ~main~, you can override this setting by a command line parameter:
@@ -30,6 +37,12 @@ Or you can set the configuration globally:
 $ git config --global changed.defaultBranch development
 #+end_src
 
+Or only for the current repository:
+
+#+begin_src bash
+$ git config changed.defaultBranch development
+#+end_src
+
 *** Filtering files with glob pattern
 
 #+begin_src bash

src/cli.rs

@@ -1,19 +1,14 @@
-use anyhow::Result;
 use clap::builder::Arg;
 use clap::builder::ArgAction;
 use clap::builder::Command;
 use clap::ArgMatches;
-use git2::Config;
 use glob::Pattern;
 use std::fmt;
 
-const CONFIG_PATHS: [&str; 2] = ["changed.defaultBranch", "init.defaultBranch"];
-const DEFAULT_BRANCH: &str = "main";
-
 #[derive(Debug, Clone)]
 pub struct Options {
     pub pattern: Pattern,
-    pub base: String,
+    pub base: Option<String>,
     pub null: bool,
     pub exist: bool,
 }
@@ -22,25 +17,12 @@ impl fmt::Display for Options {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(
             f,
-            "pattern: {}, base: {}, null: {}, exist: {}",
+            "pattern: {}, base: {:?}, null: {}, exist: {}",
             self.pattern, self.base, self.null, self.exist
         )
     }
 }
 
-fn default_branch() -> String {
-    let config = Config::open_default().expect("Failed to open global config");
-
-    let branch = CONFIG_PATHS
-        .into_iter()
-        .find_map(|path| match config.get_entry(path) {
-            Ok(entry) => entry.value().map(str::to_string),
-            Err(_e) => None,
-        });
-
-    branch.unwrap_or(DEFAULT_BRANCH.to_owned())
-}
-
 fn command() -> Command {
     Command::new("git-changed")
         .about("Lists files modified since branching off")
@@ -73,10 +55,7 @@ fn matches_to_options(matches: ArgMatches) -> Options {
 
     let pattern = Pattern::new(glob).expect("Valid glob required");
 
-    let base = match matches.get_one::<String>("base") {
-        Some(value) => value.to_owned(),
-        None => default_branch(),
-    };
+    let base = matches.get_one::<String>("base").cloned();
 
     let null = match matches.get_one::<bool>("null") {
         Some(value) => value.to_owned(),
@@ -96,8 +75,6 @@ fn matches_to_options(matches: ArgMatches) -> Options {
     }
 }
 
-pub fn get_options() -> Result<Options> {
-    let matches = command().try_get_matches()?;
-
-    Ok(matches_to_options(matches))
+pub fn get_options() -> Options {
+    matches_to_options(command().get_matches())
 }

src/git.rs

@@ -1,96 +1,143 @@
 use anyhow::Context;
 use anyhow::Result;
-use git2::BranchType;
 use git2::DiffDelta;
+use git2::ErrorCode;
 use git2::Oid;
 use git2::Repository;
 use git2::StatusOptions;
 use git2::Tree;
-use std::collections::HashSet;
-use std::env;
+use std::collections::BTreeSet;
+use std::path::Path;
+use std::path::PathBuf;
 
-fn status_options() -> StatusOptions {
-    let mut options = StatusOptions::default();
-
-    options.include_untracked(true);
-    options.exclude_submodules(true);
+const CONFIG_PATHS: [&str; 2] = ["changed.defaultBranch", "init.defaultBranch"];
+const DEFAULT_BRANCH: &str = "main";
 
-    options
+pub fn open_repository(directory: &Path) -> Result<Repository> {
+    Repository::discover(directory)
+        .with_context(|| format!("Cannot find a repository at {}", directory.display()))
 }
 
-fn uncommitted_changes(repo: &Repository) -> Result<Vec<String>> {
-    let mut options = status_options();
+pub fn subdirectory(repo: &Repository, directory: &Path) -> Result<PathBuf> {
+    let root = repo
+        .workdir()
+        .context("The repository has no working directory")?;
 
-    let statuses = repo
-        .statuses(Some(&mut options))
-        .context("Could not get statuses")?;
+    let root = canonical(root)?;
+    let directory = canonical(directory)?;
 
-    statuses
-        .iter()
-        .map(|entry| Ok(entry.path().context("Could not get path")?.to_string()))
-        .collect()
+    let subdirectory = directory
+        .strip_prefix(&root)
+        .with_context(|| format!("{} is not in {}", directory.display(), root.display()))?;
+
+    Ok(subdirectory.to_path_buf())
 }
 
-fn find_tree(repo: &Repository, oid: Oid) -> Result<Tree<'_>> {
-    repo.find_commit(oid)?
-        .tree()
-        .with_context(|| format!("Failed to find tree for {}", oid))
+pub fn changes(repo: &Repository, base: Option<&str>) -> Result<BTreeSet<String>> {
+    let base = match base {
+        Some(base) => base.to_string(),
+        None => default_branch(repo)?,
+    };
+
+    let committed = committed_changes(repo, &base)?.into_iter();
+    let uncommitted = uncommitted_changes(repo)?.into_iter();
+
+    let files = committed.chain(uncommitted);
+
+    Ok(BTreeSet::from_iter(files))
 }
 
-fn find_base_oid(repo: &Repository, base: String) -> Result<Oid> {
-    let oid = Oid::from_str(base.as_str());
+fn canonical(path: &Path) -> Result<PathBuf> {
+    path.canonicalize()
+        .with_context(|| format!("Cannot resolve the path {}", path.display()))
+}
 
-    if let Ok(value) = oid {
-        return Ok(value);
+fn default_branch(repo: &Repository) -> Result<String> {
+    let config = repo.config().context("Cannot read the git config")?;
+
+    for path in CONFIG_PATHS {
+        match config.get_string(path) {
+            Ok(branch) => return Ok(branch),
+            Err(error) if error.code() == ErrorCode::NotFound => continue,
+            Err(error) => {
+                return Err(error)
+                    .with_context(|| format!("Cannot read {path} from the git config"))
+            }
+        }
     }
 
-    repo.find_branch(&base, BranchType::Local)?
-        .get()
-        .target()
-        .with_context(|| format!("Failed to fetch branch {}", base))
+    Ok(DEFAULT_BRANCH.to_string())
 }
 
-fn committed_changes(repo: &Repository, base: &String) -> Result<Vec<String>> {
-    let base_oid = find_base_oid(repo, base.to_string())?;
+fn committed_changes(repo: &Repository, base: &str) -> Result<Vec<String>> {
+    let base_oid = find_commit_oid(repo, base)?;
+    let head_oid = find_commit_oid(repo, "HEAD")?;
 
-    let head_oid = repo.head()?.target().context("Could not get oid")?;
-
-    let base = repo
+    let merge_base_oid = repo
         .merge_base(base_oid, head_oid)
-        .with_context(|| format!("Failed to find merge base for {} and HEAD", base))?;
+        .with_context(|| format!("Cannot find a merge base of {base} and HEAD"))?;
 
-    let old_tree = find_tree(repo, base)?;
+    let old_tree = find_tree(repo, merge_base_oid)?;
     let new_tree = find_tree(repo, head_oid)?;
 
-    let diffs = repo
+    let diff = repo
         .diff_tree_to_tree(Some(&old_tree), Some(&new_tree), None)
-        .with_context(|| "Could not get the diffs")?;
+        .with_context(|| format!("Cannot compare {base} with HEAD"))?;
 
-    diffs.deltas().map(&map_new_files).collect()
+    diff.deltas().map(changed_path).collect()
 }
 
-fn map_new_files(delta: DiffDelta) -> Result<String> {
+fn find_commit_oid(repo: &Repository, revision: &str) -> Result<Oid> {
+    let commit = repo
+        .revparse_single(revision)
+        .and_then(|object| object.peel_to_commit())
+        .with_context(|| format!("Cannot find the commit of {revision}"))?;
+
+    Ok(commit.id())
+}
+
+fn find_tree(repo: &Repository, oid: Oid) -> Result<Tree<'_>> {
+    repo.find_commit(oid)
+        .and_then(|commit| commit.tree())
+        .with_context(|| format!("Cannot find the tree of {oid}"))
+}
+
+fn changed_path(delta: DiffDelta) -> Result<String> {
     let path = delta
         .new_file()
         .path()
-        .context("Failed to get file")?
+        .context("A changed file has no path")?;
+
+    let text = path
         .to_str()
-        .context("Failed to get path")?
-        .to_string();
+        .with_context(|| format!("The path {} is not UTF-8", path.display()))?;
 
-    Ok(path)
+    Ok(text.to_string())
 }
 
-pub fn changes(base: &String) -> Result<HashSet<String>> {
-    let cwd = env::current_dir()?;
+fn uncommitted_changes(repo: &Repository) -> Result<Vec<String>> {
+    let statuses = repo
+        .statuses(Some(&mut status_options()))
+        .context("Cannot read the status of the working directory")?;
 
-    let repo = Repository::open(cwd.clone())
-        .with_context(|| format!("Could not open repository at {}", cwd.display()))?;
+    statuses
+        .iter()
+        .map(|entry| {
+            let path = entry
+                .path()
+                .context("A changed file has a path that is not UTF-8")?;
 
-    let committed = committed_changes(&repo, base)?.into_iter();
-    let uncommitted = uncommitted_changes(&repo)?.into_iter();
+            Ok(path.to_string())
+        })
+        .collect()
+}
 
-    let files = committed.chain(uncommitted);
+fn status_options() -> StatusOptions {
+    let mut options = StatusOptions::default();
 
-    Ok(HashSet::from_iter(files))
+    options.include_untracked(true);
+    options.recurse_untracked_dirs(true);
+    options.exclude_submodules(true);
+
+    options
 }