cli: introduce 'ui.diff-viewer' setting

'ui.diff-viewer' combines 'ui.diff.tool' and 'ui.diff.format' into a single setting

CHANGELOG.md

@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Deprecations
 
+* `ui.diff.format` and `ui.diff.tool` were deprecated in favor of `ui.diff-viewer`
+
 * `jj move` was deprecated in favor of `jj squash`.
 
 ### Breaking changes
@@ -18,6 +20,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### New features
 
+* Add `ui.diff-viewer` combining the both `ui.diff.format` and `ui.diff.tool` into a single setting. Built-in formats previously set via `ui.diff.format` can now be specified prefixed with `:`. Having a single option allows a repository level built-in format to override a global tool format.
+
+  * `"ui.diff-viewer" = ":color-words"`
+  * `"ui.diff-viewer" = ["difft", "--color=always", "$left", "$right"]`
+
 * Config now supports rgb hex colors (in the form `#rrggbb`) wherever existing color names are supported.
 
 * `ui.default-command` now accepts multiple string arguments, for more complex

cli/src/config-schema.json

@@ -85,6 +85,19 @@
                     "description": "Pager to use for displaying command output",
                     "default": "less -FRX"
                 },
+                "diff-viewer": {
+                  "description": "Format or tool used to show diffs",
+                  "oneOf": [
+                    {
+                        "type": "string"
+                    },
+                    {
+                        "type": "array",
+                        "items": {
+                            "type": "string"
+                        }
+                    }]
+                },
                 "diff": {
                     "type": "object",
                     "description": "Options for how diffs are displayed",

cli/src/diff_util.rs

@@ -98,7 +98,7 @@ pub fn diff_formats_for(
 ) -> Result<Vec<DiffFormat>, config::ConfigError> {
     let formats = diff_formats_from_args(settings, args)?;
     if formats.is_empty() {
-        Ok(vec![default_diff_format(settings, args.context)?])
+        Ok(vec![diff_format_from_settings(settings, args.context)?])
     } else {
         Ok(formats)
     }
@@ -114,7 +114,7 @@ pub fn diff_formats_for_log(
     let mut formats = diff_formats_from_args(settings, args)?;
     // --patch implies default if no format other than --summary is specified
     if patch && matches!(formats.as_slice(), [] | [DiffFormat::Summary]) {
-        formats.push(default_diff_format(settings, args.context)?);
+        formats.push(diff_format_from_settings(settings, args.context)?);
         formats.dedup();
     }
     Ok(formats)
@@ -153,7 +153,7 @@ fn diff_formats_from_args(
 }
 
 fn builtin_diff_format(name: &str, num_context_lines: Option<usize>) -> Option<DiffFormat> {
-    match name.as_ref() {
+    match name {
         "summary" => Some(DiffFormat::Summary),
         "types" => Some(DiffFormat::Types),
         "git" => Some(DiffFormat::Git {
@@ -167,10 +167,57 @@ fn builtin_diff_format(name: &str, num_context_lines: Option<usize>) -> Option<D
     }
 }
 
-fn default_diff_format(
+fn diff_format_from_settings(
     settings: &UserSettings,
     num_context_lines: Option<usize>,
 ) -> Result<DiffFormat, config::ConfigError> {
+    let config = settings.config();
+    let format = if let Some(args) = config.get("ui.diff-viewer").optional()? {
+        if let CommandNameAndArgs::String(name) = &args {
+            // If setting is a single name starting with a colon it's treated as a built-in
+            // format. It is still possible to use an external tool starting
+            // with a colon passing it as an array rather than a string.
+            if let Some(stripped_name) = name.strip_prefix(':') {
+                Some(
+                    builtin_diff_format(stripped_name, num_context_lines).ok_or_else(|| {
+                        config::ConfigError::Message(
+                            "Unknown format setting for 'ui.diff-viewer', built-in formats are \
+                             ':summary', ':types', ':git', ':color-words', and ':stat', or use an \
+                             external tool"
+                                .to_string(),
+                        )
+                    })?,
+                )
+            } else {
+                let tool = merge_tools::get_external_tool_config(settings, name)?;
+                tool.map(|tool| DiffFormat::Tool(Box::new(tool)))
+            }
+        } else {
+            None
+        }
+        .or_else(|| {
+            Some(DiffFormat::Tool(Box::new(
+                ExternalMergeTool::with_diff_args(&args),
+            )))
+        })
+    } else {
+        Some(diff_format_from_deprecated_settings(
+            settings,
+            num_context_lines,
+        )?)
+    };
+
+    Ok(format.unwrap_or(DiffFormat::ColorWords {
+        context: num_context_lines.unwrap_or(DEFAULT_CONTEXT_LINES),
+    }))
+}
+
+fn diff_format_from_deprecated_settings(
+    settings: &UserSettings,
+    num_context_lines: Option<usize>,
+) -> Result<DiffFormat, config::ConfigError> {
+    // ui.diff.tool and ui.diff.format are deprecated in favour of the combined
+    // ui.diff-viewer
     let config = settings.config();
     if let Some(args) = config.get("ui.diff.tool").optional()? {
         // External "tool" overrides the internal "format" option.

cli/tests/test_diff_command.rs

@@ -740,6 +740,93 @@ fn test_diff_skipped_context_nondefault() {
     "###);
 }
 
+#[test]
+fn test_diff_formatter_setting() {
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let repo_path = test_env.env_root().join("repo");
+
+    std::fs::write(repo_path.join("file1"), "foo\n").unwrap();
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':summary'"]), @r###"
+    A file1
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':types'"]), @r###"
+    -F file1
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':git'"]), @r###"
+    diff --git a/file1 b/file1
+    new file mode 100644
+    index 0000000000..257cc5642c
+    --- /dev/null
+    +++ b/file1
+    @@ -1,0 +1,1 @@
+    +foo
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':color-words'"]), @r###"
+    Added regular file file1:
+            1: foo
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':stat'"]), @r###"
+    file1 | 1 +
+    1 file changed, 1 insertion(+), 0 deletions(-)
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_failure(&repo_path, &["diff", "--config-toml=ui.diff-viewer=':unknown'"]), @r###"
+    Config error: Unknown format setting for 'ui.diff-viewer', built-in formats are ':summary', ':types', ':git', ':color-words', and ':stat', or use an external tool
+    For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md.
+    "###);
+
+    let edit_script = test_env.set_up_fake_diff_editor();
+    std::fs::write(
+        edit_script,
+        "print-files-before\0print --\0print-files-after",
+    )
+    .unwrap();
+
+    let command = escaped_fake_diff_editor_path();
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", "--config-toml=ui.diff-viewer='fake-diff-editor'"]), @r###"
+    --
+    file1
+    "###);
+
+    insta::assert_snapshot!(
+      test_env.jj_cmd_success(&repo_path, &["diff", &format!(r#"--config-toml=ui.diff-viewer=["{command}"]"#)]), @r###"
+    --
+    file1
+    "###);
+
+    if cfg!(windows) {
+        insta::assert_snapshot!(
+      test_env.jj_cmd_failure(&repo_path, &["diff", r#"--config-toml=ui.diff-viewer=["unknown-diff-binary"]"#]), @r###"
+    Error: Failed to generate diff
+    Caused by:
+    1: Error executing 'unknown-diff-binary' (run with --debug to see the exact invocation)
+    2: program not found
+    "###);
+    } else {
+        insta::assert_snapshot!(
+      test_env.jj_cmd_failure(&repo_path, &["diff", r#"--config-toml=ui.diff-viewer=["unknown-diff-binary"]"#]), @r###"
+    Error: Failed to generate diff
+    Caused by:
+    1: Error executing 'unknown-diff-binary' (run with --debug to see the exact invocation)
+    2: No such file or directory (os error 2)
+    "###);
+    }
+}
+
 #[test]
 fn test_diff_external_tool() {
     let mut test_env = TestEnvironment::default();
@@ -812,7 +899,7 @@ fn test_diff_external_tool() {
     "###);
 
     // Enabled by default, looks up the merge-tools table
-    let config = "--config-toml=ui.diff.tool='fake-diff-editor'";
+    let config = "--config-toml=ui.diff-viewer='fake-diff-editor'";
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", config]), @r###"
     file1
     file2
@@ -823,7 +910,7 @@ fn test_diff_external_tool() {
 
     // Inlined command arguments
     let command = escaped_fake_diff_editor_path();
-    let config = format!(r#"--config-toml=ui.diff.tool=["{command}", "$right", "$left"]"#);
+    let config = format!(r#"--config-toml=ui.diff-viewer=["{command}", "$right", "$left"]"#);
     insta::assert_snapshot!(test_env.jj_cmd_success(&repo_path, &["diff", &config]), @r###"
     file2
     file3

cli/tests/test_log_command.rs

@@ -138,7 +138,7 @@ fn test_log_with_or_without_diff() {
             "description",
             "-p",
             "-s",
-            "--config-toml=ui.diff.format='summary'",
+            "--config-toml=ui.diff-viewer=':summary'",
         ],
     );
     insta::assert_snapshot!(stdout, @r###"

docs/config.md

@@ -165,24 +165,23 @@ useful reminder to fill in things like BUG=, TESTED= etc.
 ui.default-description = "\n\nTESTED=TODO"
 ```
 
-### Diff format
+### Generating diffs with built-in and external formatters
+
+Different diff formatters can be chosen with the `ui.diff-viewer` setting. `jj` provides three built-in formatters, `:color-words` (the default), `:git`, and `:summary`.
 
 ```toml
-# Possible values: "color-words" (default), "git", "summary"
-ui.diff.format = "git"
+[ui]
+diff-viewer = ":git"
 ```
 
-### Generating diffs by external command
-
-If `ui.diff.tool` is set, the specified diff command will be called instead of
-the internal diff function.
+An external diff command can be used instead of a built-in formatter, specifying the command and its arguments.
 
 ```toml
 [ui]
 # Use Difftastic by default
-diff.tool = ["difft", "--color=always", "$left", "$right"]
+diff-viewer = ["difft", "--color=always", "$left", "$right"]
 # Use tool named "<name>" (see below)
-diff.tool = "<name>"
+diff-viewer = "<name>"
 ```
 
 The external diff tool can also be enabled by `diff --tool <name>` argument.