Repository: dandavison/delta
Pull request #697 (2021-08-21 02:55:59)

Use "syntax theme" terminology in show-syntax-themes output
Ref #678


Patch:
diff --git a/src/cli.rs b/src/cli.rs
index 1b88358..b48d52f 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -13,7 +13,7 @@ use crate::bat_utils::output::PagingMode;
 use crate::git_config::{GitConfig, GitConfigEntry};
 use crate::options;
 
-#[derive(StructOpt, Clone, Default)]
+#[derive(StructOpt, Default)]
 #[structopt(
     name = "delta",
     about = "A viewer for git and diff output",
@@ -629,6 +629,9 @@ pub struct Opt {
     #[structopt(skip)]
     pub computed: ComputedValues,
 
+    #[structopt(skip)]
+    pub git_config: Option<GitConfig>,
+
     #[structopt(skip)]
     pub git_config_entries: HashMap<String, GitConfigEntry>,
 }
@@ -678,13 +681,13 @@ impl Default for PagingMode {
 
 impl Opt {
     pub fn from_args_and_git_config(
-        git_config: &mut Option<GitConfig>,
+        git_config: Option<GitConfig>,
         assets: HighlightingAssets,
     ) -> Self {
         Self::from_clap_and_git_config(Self::clap().get_matches(), git_config, assets)
     }
 
-    pub fn from_iter_and_git_config<I>(iter: I, git_config: &mut Option<GitConfig>) -> Self
+    pub fn from_iter_and_git_config<I>(iter: I, git_config: Option<GitConfig>) -> Self
     where
         I: IntoIterator,
         I::Item: Into<OsString> + Clone,
@@ -695,12 +698,13 @@ impl Opt {
 
     fn from_clap_and_git_config(
         arg_matches: clap::ArgMatches,
-        git_config: &mut Option<GitConfig>,
+        mut git_config: Option<GitConfig>,
         assets: HighlightingAssets,
     ) -> Self {
         let mut opt = Opt::from_clap(&arg_matches);
         options::rewrite::apply_rewrite_rules(&mut opt, &arg_matches);
-        options::set::set_options(&mut opt, git_config, &arg_matches, assets);
+        options::set::set_options(&mut opt, &mut git_config, &arg_matches, assets);
+        opt.git_config = git_config;
         opt
     }
 
diff --git a/src/config.rs b/src/config.rs
index 1972fa4..b0a8cb3 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -15,7 +15,7 @@ use crate::delta::State;
 use crate::env;
 use crate::features::navigate;
 use crate::features::side_by_side;
-use crate::git_config::GitConfigEntry;
+use crate::git_config::{GitConfig, GitConfigEntry};
 use crate::style::{self, Style};
 
 pub struct Config {
@@ -35,6 +35,7 @@ pub struct Config {
     pub file_renamed_label: String,
     pub hunk_label: String,
     pub file_style: Style,
+    pub git_config: Option<GitConfig>,
     pub git_config_entries: HashMap<String, GitConfigEntry>,
     pub hunk_header_file_style: Style,
     pub hunk_header_line_number_style: Style,
@@ -213,6 +214,7 @@ impl From<cli::Opt> for Config {
             file_renamed_label,
             hunk_label,
             file_style,
+            git_config: opt.git_config,
             git_config_entries: opt.git_config_entries,
             hunk_header_file_style,
             hunk_header_line_number_style,
diff --git a/src/features/hyperlinks.rs b/src/features/hyperlinks.rs
index bf0c235..2d3ad3b 100644
--- a/src/features/hyperlinks.rs
+++ b/src/features/hyperlinks.rs
@@ -1,11 +1,12 @@
 use std::borrow::Cow;
+use std::str::FromStr;
 
 use lazy_static::lazy_static;
 use regex::{Captures, Regex};
 
 use crate::config::Config;
 use crate::features::OptionValueFunction;
-use crate::git_config::{GitConfigEntry, GitRemoteRepo};
+use crate::git_config::{GitConfig, GitConfigEntry, GitRemoteRepo};
 
 pub fn make_feature() -> Vec<(String, OptionValueFunction)> {
     builtin_feature!([
@@ -28,16 +29,30 @@ pub fn format_commit_line_with_osc8_commit_hyperlink<'a>(
             format_osc8_hyperlink(&commit_link_format.replace("{commit}", commit), commit)
         })
     } else if let Some(GitConfigEntry::GitRemote(GitRemoteRepo::GitHubRepo(repo))) =
-        config.git_config_entries.get("remote.origin.url")
+        config.git_config.as_ref().and_then(get_remote_url)
     {
         COMMIT_LINE_REGEX.replace(line, |captures: &Captures| {
-            format_commit_line_captures_with_osc8_commit_hyperlink(captures, repo)
+            format_commit_line_captures_with_osc8_commit_hyperlink(captures, &repo)
         })
     } else {
         Cow::from(line)
     }
 }
 
+fn get_remote_url(git_config: &GitConfig) -> Option<GitConfigEntry> {
+    git_config
+        .repo
+        .as_ref()?
+        .find_remote("origin")
+        .ok()?
+        .url()
+        .and_then(|url| {
+            GitRemoteRepo::from_str(url)
+                .ok()
+                .map(GitConfigEntry::GitRemote)
+        })
+}
+
 /// Create a file hyperlink to `path`, displaying `text`.
 pub fn format_osc8_file_hyperlink<'a>(
     relative_path: &'a str,
diff --git a/src/features/mod.rs b/src/features/mod.rs
index 1fe2091..8b169fc 100644
--- a/src/features/mod.rs
+++ b/src/features/mod.rs
@@ -102,7 +102,7 @@ pub mod tests {
         let builtin_features = make_builtin_features();
         let mut args = vec!["delta".to_string()];
         args.extend(builtin_features.keys().map(|s| format!("--{}", s)));
-        let opt = cli::Opt::from_iter_and_git_config(args, &mut None);
+        let opt = cli::Opt::from_iter_and_git_config(args, None);
         let features: HashSet<&str> = opt.features.split_whitespace().collect();
         for feature in builtin_features.keys() {
             assert!(features.contains(feature.as_str()))
diff --git a/src/main.rs b/src/main.rs
index 1b77808..ccf8266 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -58,7 +58,7 @@ pub mod errors {
 // arguments and without standard input; 2 is used to report a real problem.
 fn run_app() -> std::io::Result<i32> {
     let assets = HighlightingAssets::new();
-    let opt = cli::Opt::from_args_and_git_config(&mut git_config::GitConfig::try_create(), assets);
+    let opt = cli::Opt::from_args_and_git_config(git_config::GitConfig::try_create(), assets);
 
     if opt.list_languages {
         list_languages()?;
@@ -159,19 +159,8 @@ You can also use delta to diff two files: `delta file_A file_B`."
     }
     let mut diff_process = diff_process.unwrap();
 
-    let exit_code = diff_process
-        .wait()
-        .unwrap_or_else(|_| {
-            delta_unreachable(&format!("'{}' process not running.", diff_command));
-        })
-        .code()
-        .unwrap_or_else(|| {
-            eprintln!("'{}' process terminated without exit status.", diff_command);
-            config.error_exit_code
-        });
-
     if let Err(error) = delta(
-        BufReader::new(diff_process.stdout.unwrap()).byte_lines(),
+        BufReader::new(diff_process.stdout.take().unwrap()).byte_lines(),
         writer,
         config,
     ) {
@@ -183,7 +172,20 @@ You can also use delta to diff two files: `delta file_A file_B`."
             }
         }
     };
-    exit_code
+
+    // Return the exit code from the `git diff` processl, so that the exit code
+    // contract of `delta file_A file_B` is the same as that of `diff file_A
+    // file_B` (i.e. 0 if same, 1 if different, 2 if error).
+    diff_process
+        .wait()
+        .unwrap_or_else(|_| {
+            delta_unreachable(&format!("'{}' process not running.", diff_command));
+        })
+        .code()
+        .unwrap_or_else(|| {
+            eprintln!("'{}' process terminated without exit status.", diff_command);
+            config.error_exit_code
+        })
 }
 
 fn show_config(config: &config::Config, writer: &mut dyn Write) -> std::io::Result<()> {
@@ -351,19 +353,17 @@ fn show_themes(dark: bool, light: bool, computed_theme_is_light: bool) -> std::i
         }
     };
 
-    let mut git_config = git_config::GitConfig::try_create();
-    let opt = cli::Opt::from_iter_and_git_config(
-        &["", "", "--navigate", "--show-themes"],
-        &mut git_config,
-    );
+    let git_config = git_config::GitConfig::try_create();
+    let opt =
+        cli::Opt::from_iter_and_git_config(&["", "", "--navigate", "--show-themes"], git_config);
     let mut output_type =
         OutputType::from_mode(PagingMode::Always, None, &config::Config::from(opt)).unwrap();
     let title_style = ansi_term::Style::new().bold();
     let writer = output_type.handle().unwrap();
 
     for theme in &get_themes(git_config::GitConfig::try_create()) {
-        let opt =
-            cli::Opt::from_iter_and_git_config(&["", "", "--features", theme], &mut git_config);
+        let git_config = git_config::GitConfig::try_create();
+        let opt = cli::Opt::from_iter_and_git_config(&["", "", "--features", theme], git_config);
         let is_dark_theme = opt.dark;
         let is_light_theme = opt.light;
         let config = config::Config::from(opt);
@@ -389,7 +389,6 @@ fn show_themes(dark: bool, light: bool, computed_theme_is_light: bool) -> std::i
 
 #[cfg(not(tarpaulin_include))]
 fn show_syntax_themes() -> std::io::Result<()> {
-    let mut opt = cli::Opt::from_args();
     let assets = HighlightingAssets::new();
     let mut output_type = OutputType::from_mode(
         PagingMode::QuitIfOneScreen,
@@ -398,7 +397,6 @@ fn show_syntax_themes() -> std::io::Result<()> {
     )
     .unwrap();
     let mut writer = output_type.handle().unwrap();
-    opt.computed.syntax_set = assets.syntax_set;
 
     let stdin_data = if !atty::is(atty::Stream::Stdin) {
         let mut buf = Vec::new();
@@ -412,9 +410,16 @@ fn show_syntax_themes() -> std::io::Result<()> {
         None
     };
 
+    let make_opt = || {
+        let mut opt = cli::Opt::from_args();
+        opt.computed.syntax_set = assets.syntax_set.clone();
+        opt
+    };
+    let opt = make_opt();
+
     if !(opt.dark || opt.light) {
-        _show_syntax_themes(opt.clone(), false, &mut writer, stdin_data.as_ref())?;
-        _show_syntax_themes(opt, true, &mut writer, stdin_data.as_ref())?;
+        _show_syntax_themes(opt, false, &mut writer, stdin_data.as_ref())?;
+        _show_syntax_themes(make_opt(), true, &mut writer, stdin_data.as_ref())?;
     } else if opt.light {
         _show_syntax_themes(opt, true, &mut writer, stdin_data.as_ref())?;
     } else {
@@ -464,7 +469,11 @@ index f38589a..0f1bb83 100644
         .filter(|(t, _)| is_light_syntax_theme(t) == is_light_mode)
         .map(|(t, _)| t)
     {
-        writeln!(writer, "\n\nTheme: {}\n", title_style.paint(syntax_theme))?;
+        writeln!(
+            writer,
+            "\n\nSyntax theme: {}\n",
+            title_style.paint(syntax_theme)
+        )?;
         config.syntax_theme = Some(assets.theme_set.themes[syntax_theme.as_str()].clone());
         if let Err(error) = delta(ByteLines::new(BufReader::new(&input[0..])), writer, &config) {
             match error.kind() {
@@ -491,11 +500,11 @@ pub fn _list_syntax_themes_for_humans(writer: &mut dyn Write) -> std::io::Result
     let assets = HighlightingAssets::new();
     let themes = &assets.theme_set.themes;
 
-    writeln!(writer, "Light themes:")?;
+    writeln!(writer, "Light syntax themes:")?;
     for (theme, _) in themes.iter().filter(|(t, _)| is_light_syntax_theme(*t)) {
         writeln!(writer, "    {}", theme)?;
     }
-    writeln!(writer, "\nDark themes:")?;
+    writeln!(writer, "\nDark syntax themes:")?;
     for (theme, _) in themes.iter().filter(|(t, _)| !is_light_syntax_theme(*t)) {
         writeln!(writer, "    {}", theme)?;
     }
@@ -559,7 +568,7 @@ mod main_tests {
         writer.seek(SeekFrom::Start(0)).unwrap();
         writer.read_to_string(&mut s).unwrap();
         let s = ansi::strip_ansi_codes(&s);
-        assert!(s.contains("\nTheme: gruvbox-light\n"));
+        assert!(s.contains("\nSyntax theme: gruvbox-light\n"));
         println!("{}", s);
         assert!(s.contains("\nfn print_cube(num: f64) {\n"));
     }
@@ -571,9 +580,9 @@ mod main_tests {
         let mut s = String::new();
         writer.seek(SeekFrom::Start(0)).unwrap();
         writer.read_to_string(&mut s).unwrap();
-        assert!(s.contains("Light themes:\n"));
+        assert!(s.contains("Light syntax themes:\n"));
         assert!(s.contains("    GitHub\n"));
-        assert!(s.contains("Dark themes:\n"));
+        assert!(s.contains("Dark syntax themes:\n"));
         assert!(s.contains("    Dracula\n"));
     }
 
diff --git a/src/options/set.rs b/src/options/set.rs
index 4fc0a1f..53240fc 100644
--- a/src/options/set.rs
+++ b/src/options/set.rs
@@ -13,7 +13,7 @@ use crate::config;
 use crate::env;
 use crate::errors::*;
 use crate::features;
-use crate::git_config::{GitConfig, GitConfigEntry, GitRemoteRepo};
+use crate::git_config::{GitConfig, GitConfigEntry};
 use crate::options::option_value::{OptionValue, ProvenancedOptionValue};
 use crate::options::theme;
 
@@ -564,7 +564,6 @@ fn is_truecolor_terminal() -> bool {
 }
 
 fn set_git_config_entries(opt: &mut cli::Opt, git_config: &mut GitConfig) {
-    // Styles
     for key in &["color.diff.old", "color.diff.new"] {
         if let Some(style_string) = git_config.get::<String>(key) {
             opt.git_config_entries
@@ -572,16 +571,6 @@ fn set_git_config_entries(opt: &mut cli::Opt, git_config: &mut GitConfig) {
         }
     }
 
-    // Strings
-    for key in &["remote.origin.url"] {
-        if let Some(string) = git_config.get::<String>(key) {
-            if let Ok(repo) = GitRemoteRepo::from_str(&string) {
-                opt.git_config_entries
-                    .insert(key.to_string(), GitConfigEntry::GitRemote(repo));
-            }
-        }
-    }
-
     if let Some(repo) = &git_config.repo {
         if let Some(workdir) = repo.workdir() {
             opt.git_config_entries.insert(
diff --git a/src/paint.rs b/src/paint.rs
index 3b5010b..0e56923 100644
--- a/src/paint.rs
+++ b/src/paint.rs
@@ -381,9 +381,9 @@ impl<'a> Painter<'a> {
         painted_prefix: Option<ansi_term::ANSIString>,
         config: &config::Config,
     ) -> (String, bool) {
-        let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
-        let mut handled_prefix = false;
         let mut ansi_strings = Vec::new();
+
+        let output_line_numbers = config.line_numbers && line_numbers_data.is_some();
         if output_line_numbers {
             ansi_strings.extend(line_numbers::format_and_paint_line_numbers(
                 line_numbers_data.as_mut().unwrap(),
@@ -407,27 +407,40 @@ impl<'a> Painter<'a> {
             }
             _ => {}
         }
-        let mut is_empty = true;
-        for (section_style, mut text) in superimpose_style_sections(
+
+        let superimposed = superimpose_style_sections(
             syntax_sections,
             diff_sections,
             config.true_color,
             config.null_syntect_style,
-        ) {
-            if !handled_prefix {
-                if let Some(painted_prefix) = painted_prefix.clone() {
-                    ansi_strings.push(painted_prefix);
+        );
+
+        let mut handled_prefix = false;
+        for (section_style, text) in &superimposed {
+            let text = if handled_prefix {
+                &text
+            } else {
+                // Remove what was originally the +/- prefix, see `prepare()`, after
+                // (if requested) re-inserting it with proper styling.
+                if let Some(ref painted_prefix) = painted_prefix {
+                    ansi_strings.push(painted_prefix.clone());
                 }
+
                 if !text.is_empty() {
-                    text.remove(0);
+                    &text[1..]
+                } else {
+                    &text
                 }
-                handled_prefix = true;
-            }
+            };
+
             if !text.is_empty() {
                 ansi_strings.push(section_style.paint(text));
-                is_empty = false;
             }
+            handled_prefix = true;
         }
+
+        // Only if syntax is empty (implies diff empty) can a line actually be empty.
+        let is_empty = syntax_sections.is_empty();
         (ansi_term::ANSIStrings(&ansi_strings).to_string(), is_empty)
     }
 


Test patch:
diff --git a/src/tests/integration_test_utils.rs b/src/tests/integration_test_utils.rs
index 7cab6f8..bdb2851 100644
--- a/src/tests/integration_test_utils.rs
+++ b/src/tests/integration_test_utils.rs
@@ -38,14 +38,14 @@ fn _make_options_from_args_and_git_config(
     let mut args: Vec<&str> = itertools::chain(&["/dev/null", "/dev/null"], args)
         .map(|s| *s)
         .collect();
-    let mut git_config = match (git_config_contents, git_config_path) {
+    let git_config = match (git_config_contents, git_config_path) {
         (Some(contents), Some(path)) => Some(make_git_config(contents, path, honor_env_var)),
         _ => {
             args.push("--no-gitconfig");
             None
         }
     };
-    cli::Opt::from_iter_and_git_config(args, &mut git_config)
+    cli::Opt::from_iter_and_git_config(args, git_config)
 }
 
 pub fn make_options_from_args(args: &[&str]) -> cli::Opt {
diff --git a/src/tests/test_example_diffs.rs b/src/tests/test_example_diffs.rs
index c605715..3f28d20 100644
--- a/src/tests/test_example_diffs.rs
+++ b/src/tests/test_example_diffs.rs
@@ -1472,14 +1472,7 @@ src/align.rs:71: impl<'a> Alignment<'a> { │
                     .to_string()
             );
         } else {
-            let style = style::Style::from_str(empty_line_marker_style, None, None, true, false);
-            assert_eq!(
-                line,
-                &style
-                    .ansi_term_style
-                    .paint(ansi::ANSI_CSI_CLEAR_TO_BOL)
-                    .to_string()
-            );
+            assert_eq!(line, "");
         }
     }