UX improvements (#28)

* Allow setting the diff theme
* Read configuration from git config as well as arguments and env vars
* Choose whether to display full diff or just a diffstat based on
terminal
  height instead of a constant
* Add -u alias for --default-upstream-branch

Author: quodlibetor

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Unreleased
 
+* Allow setting the diff theme
+* Read configuration from git config as well as arguments and env vars
+* Choose whether to display full diff or just a diffstat based on terminal
+  height instead of a constant
+* Add -u alias for --default-upstream-branch
+
 # Version 0.2.2
 
 * Correctly retarget branches if the target of the edit is also a branch (#24)

Cargo.lock

@@ -21,12 +21,14 @@ clap = { version = "4.5.1", features = ["derive", "env", "wrap_help"] }
 console = "0.15.8"
 dialoguer = "0.11.0"
 git2 = { version = "0.18.2", default-features = false }
+itertools = "0.12.1"
+termcolor = "1.4.1"
+terminal_size = "0.3.0"
 syntect = "5.2.0"
 
 [dev-dependencies]
 assert_cmd = "2.0.13"
 assert_fs = "1.1.1"
-itertools = "0.12.1"
 
 # The profile that 'cargo dist' will build with
 [profile.dist]

Cargo.toml

@@ -1,4 +1,5 @@
 use std::collections::HashMap;
+use std::io::Write as _;
 
 use anyhow::{anyhow, bail, Context};
 use console::style;
@@ -11,26 +12,43 @@ use syntect::easy::HighlightLines;
 use syntect::highlighting::ThemeSet;
 use syntect::parsing::SyntaxSet;
 use syntect::util::as_24_bit_terminal_escaped;
+use termcolor::{ColorChoice, StandardStream, WriteColor as _};
+use terminal_size::{terminal_size, Height};
 
 const DEFAULT_UPSTREAM_BRANCHES: &[&str] = &["main", "master", "develop", "trunk"];
+pub const DEFAULT_THEME: &str = "base16-ocean.dark";
+
+pub struct Config {
+    /// Change the commit message that you amend, instead of using the original commit message
+    pub squash: bool,
+    /// The maximum number of commits to show when looking for your merge point
+    pub max_commits: usize,
+    /// Specify a commit to ammend by the subject line of the commit
+    pub commit_message_pattern: Option<String>,
+    pub default_upstream_branch: Option<String>,
+    /// Require a newline when confirming y/n questions
+    pub require_newline: bool,
+    /// Which theme to use
+    pub theme: String,
+}
 
-pub fn instafix(
-    squash: bool,
-    max_commits: usize,
-    message_pattern: Option<String>,
-    upstream_branch_name: Option<&str>,
-    require_newline: bool,
-) -> Result<(), anyhow::Error> {
+pub fn instafix(c: Config) -> Result<(), anyhow::Error> {
     let repo = Repository::open(".").context("opening repo")?;
-    let diff = create_diff(&repo, require_newline).context("creating diff")?;
+    let diff = create_diff(&repo, &c.theme, c.require_newline).context("creating diff")?;
     let head = repo.head().context("finding head commit")?;
     let head_branch = Branch::wrap(head);
-    let upstream =
-        get_merge_base(&repo, &head_branch, upstream_branch_name).context("creating merge base")?;
-    let commit_to_amend = select_commit_to_amend(&repo, upstream, max_commits, &message_pattern)
-        .context("selecting commit to amend")?;
+    let upstream = get_merge_base(&repo, &head_branch, c.default_upstream_branch.as_deref())
+        .context("creating merge base")?;
+    let commit_to_amend = select_commit_to_amend(
+        &repo,
+        upstream,
+        c.max_commits,
+        c.commit_message_pattern.as_deref(),
+    )
+    .context("selecting commit to amend")?;
     eprintln!("Selected {}", disp(&commit_to_amend));
-    do_fixup_commit(&repo, &head_branch, &commit_to_amend, squash).context("doing fixup commit")?;
+    do_fixup_commit(&repo, &head_branch, &commit_to_amend, c.squash)
+        .context("doing fixup commit")?;
     let needs_stash = worktree_is_dirty(&repo)?;
     if needs_stash {
         // TODO: is it reasonable to create a new repo to work around lifetime issues?
@@ -239,7 +257,11 @@ fn get_merge_base<'a>(
 }
 
 /// Get a diff either from the index or the diff from the index to the working tree
-fn create_diff(repo: &Repository, require_newline: bool) -> Result<Diff, anyhow::Error> {
+fn create_diff<'a>(
+    repo: &'a Repository,
+    theme: &str,
+    require_newline: bool,
+) -> Result<Diff<'a>, anyhow::Error> {
     let head = repo.head()?;
     let head_tree = head.peel_to_tree()?;
     let staged_diff = repo.diff_tree_to_index(Some(&head_tree), None, None)?;
@@ -248,11 +270,18 @@ fn create_diff(repo: &Repository, require_newline: bool) -> Result<Diff, anyhow:
     let diff = if diffstat.files_changed() == 0 {
         let dirty_workdir_stats = dirty_diff.stats()?;
         if dirty_workdir_stats.files_changed() > 0 {
+            let Height(h) = terminal_size().map(|(_w, h)| h).unwrap_or(Height(24));
+            let cutoff_height = (h - 5) as usize; // give some room for the prompt
             let total_change = dirty_workdir_stats.insertions() + dirty_workdir_stats.deletions();
-            if total_change < 50 {
-                native_diff(&dirty_diff)?;
-            } else {
+            if total_change >= cutoff_height {
                 print_diffstat("Unstaged", &dirty_diff)?;
+            } else {
+                let diff_lines = native_diff(&dirty_diff, theme)?;
+                if diff_lines.len() >= cutoff_height {
+                    print_diffstat("Unstaged", &dirty_diff)?;
+                } else {
+                    print_diff_lines(&diff_lines)?;
+                }
             }
             if !Confirm::new()
                 .with_prompt("Nothing staged, stage and commit everything?")
@@ -312,7 +341,7 @@ fn select_commit_to_amend<'a>(
     repo: &'a Repository,
     upstream: Option<Object<'a>>,
     max_commits: usize,
-    message_pattern: &Option<String>,
+    message_pattern: Option<&str>,
 ) -> Result<Commit<'a>, anyhow::Error> {
     let mut walker = repo.revwalk()?;
     walker.push_head()?;
@@ -417,15 +446,29 @@ fn format_ref(rf: &git2::Reference<'_>) -> Result<String, anyhow::Error> {
     Ok(format!("{} ({})", shorthand, &sha[..10]))
 }
 
+/// A vec of all built-in theme names
+pub fn print_themes() {
+    println!("Available themes:");
+    for theme in ThemeSet::load_defaults().themes.keys() {
+        println!("  {}", theme);
+    }
+}
+
 // diff helpers
 
-fn native_diff(diff: &Diff<'_>) -> Result<(), anyhow::Error> {
+fn native_diff(diff: &Diff<'_>, theme: &str) -> Result<Vec<String>, anyhow::Error> {
     let ss = SyntaxSet::load_defaults_newlines();
     let ts = ThemeSet::load_defaults();
     let syntax = ss.find_syntax_by_extension("patch").unwrap();
-    let mut h = HighlightLines::new(syntax, &ts.themes["base16-ocean.dark"]);
+    let mut h = HighlightLines::new(
+        syntax,
+        ts.themes
+            .get(theme)
+            .unwrap_or_else(|| &ts.themes[DEFAULT_THEME]),
+    );
 
     let mut inner_err = None;
+    let mut diff_lines = Vec::new();
 
     diff.print(DiffFormat::Patch, |_delta, _hunk, line| {
         let content = std::str::from_utf8(line.content()).unwrap();
@@ -441,7 +484,7 @@ fn native_diff(diff: &Diff<'_>) -> Result<(), anyhow::Error> {
                     }
                 };
                 let escaped = as_24_bit_terminal_escaped(&ranges[..], true);
-                print!("{}", escaped);
+                diff_lines.push(escaped);
             }
             _ => {
                 let ranges = match h.highlight_line(content, &ss) {
@@ -452,7 +495,7 @@ fn native_diff(diff: &Diff<'_>) -> Result<(), anyhow::Error> {
                     }
                 };
                 let escaped = as_24_bit_terminal_escaped(&ranges[..], true);
-                print!("{}", escaped);
+                diff_lines.push(escaped);
             }
         }
         true
@@ -461,10 +504,20 @@ fn native_diff(diff: &Diff<'_>) -> Result<(), anyhow::Error> {
     if let Some(err) = inner_err {
         Err(err.into())
     } else {
-        Ok(())
+        Ok(diff_lines)
     }
 }
 
+fn print_diff_lines(diff_lines: &[String]) -> Result<(), anyhow::Error> {
+    let mut stdout = StandardStream::stdout(ColorChoice::Auto);
+    for line in diff_lines {
+        write!(&mut stdout, "{}", line)?;
+    }
+    stdout.reset()?;
+    writeln!(&mut stdout)?;
+    Ok(())
+}
+
 fn print_diffstat(prefix: &str, diff: &Diff<'_>) -> Result<(), anyhow::Error> {
     let buf = diff.stats()?.to_buf(DiffStatsFormat::FULL, 80)?;
     let stat = std::str::from_utf8(&buf).context("converting diffstat to utf-8")?;

src/lib.rs

@@ -15,9 +15,12 @@
 use std::env;
 
 use clap::Parser;
+use git_instafix::DEFAULT_THEME;
 
+const MAX_COMMITS_VAR: &str = "GIT_INSTAFIX_MAX_COMMITS";
 const UPSTREAM_VAR: &str = "GIT_INSTAFIX_UPSTREAM";
-const REQUIRE_NEWLINE: &str = "GIT_INSTAFIX_REQUIRE_NEWLINE";
+const REQUIRE_NEWLINE_VAR: &str = "GIT_INSTAFIX_REQUIRE_NEWLINE";
+const THEME_VAR: &str = "GIT_INSTAFIX_THEME";
 
 #[derive(Parser, Debug)]
 #[clap(
@@ -39,36 +42,50 @@ When run with no arguments this will:
 )]
 struct Args {
     /// Change the commit message that you amend, instead of using the original commit message
-    #[clap(short = 's', long = "squash")]
-    squash: bool,
+    #[clap(short = 's', long, hide = true)]
+    squash: Option<bool>,
     /// The maximum number of commits to show when looking for your merge point
-    #[clap(short = 'm', long = "max-commits", default_value = "15")]
-    max_commits: usize,
+    ///
+    /// [gitconfig: instafix.max-commits]
+    #[clap(short = 'm', long = "max-commits", env = MAX_COMMITS_VAR)]
+    max_commits: Option<usize>,
 
     /// Specify a commit to ammend by the subject line of the commit
     #[clap(short = 'P', long)]
     commit_message_pattern: Option<String>,
 
-    #[clap(long, env = UPSTREAM_VAR)]
+    /// The branch to not go past when looking for your merge point
+    ///
+    /// [gitconfig: instafix.default-upstream-branch]
+    #[clap(short = 'u', long, env = UPSTREAM_VAR)]
     default_upstream_branch: Option<String>,
 
     /// Require a newline when confirming y/n questions
-    #[clap(long, env = REQUIRE_NEWLINE)]
-    require_newline: bool,
+    ///
+    /// [gitconfig: instafix.require-newline]
+    #[clap(long, env = REQUIRE_NEWLINE_VAR)]
+    require_newline: Option<bool>,
+
+    /// Show the possible color themes for output
+    #[clap(long)]
+    help_themes: bool,
+
+    /// Use this theme
+    #[clap(long, env = THEME_VAR)]
+    theme: Option<String>,
 }
 
 fn main() {
     let mut args = Args::parse();
     if env::args().next().unwrap().ends_with("squash") {
-        args.squash = true
+        args.squash = Some(true)
+    }
+    if args.help_themes {
+        git_instafix::print_themes();
+        return;
     }
-    if let Err(e) = git_instafix::instafix(
-        args.squash,
-        args.max_commits,
-        args.commit_message_pattern,
-        args.default_upstream_branch.as_deref(),
-        args.require_newline,
-    ) {
+    let config = args_to_config_using_git_config(args).unwrap();
+    if let Err(e) = git_instafix::instafix(config) {
         // An empty message means don't display any error message
         let msg = e.to_string();
         if !msg.is_empty() {
@@ -81,3 +98,28 @@ fn main() {
         std::process::exit(1);
     }
 }
+
+fn args_to_config_using_git_config(args: Args) -> Result<git_instafix::Config, anyhow::Error> {
+    let mut cfg = git2::Config::open_default()?;
+    let repo = git2::Repository::discover(".")?;
+    cfg.add_file(&repo.path().join("config"), git2::ConfigLevel::Local, false)?;
+    Ok(git_instafix::Config {
+        squash: args
+            .squash
+            .unwrap_or_else(|| cfg.get_bool("instafix.squash").unwrap_or(false)),
+        max_commits: args
+            .max_commits
+            .unwrap_or_else(|| cfg.get_i32("instafix.max-commits").unwrap_or(15) as usize),
+        commit_message_pattern: args.commit_message_pattern,
+        default_upstream_branch: args
+            .default_upstream_branch
+            .or_else(|| cfg.get_string("instafix.default-upstream-branch").ok()),
+        require_newline: args
+            .require_newline
+            .unwrap_or_else(|| cfg.get_bool("instafix.require-newline").unwrap_or(false)),
+        theme: args.theme.unwrap_or_else(|| {
+            cfg.get_string("instafix.theme")
+                .unwrap_or_else(|_| DEFAULT_THEME.to_string())
+        }),
+    })
+}