cli: add option to generate textual diff by external command

This is basic implementation. There's no config knob to enable the external
diff command by default. It reuses the merge-tools table because that's how
external diff/merge commands are currently configured. We might want to
reorganize them in jj-vcs#1285.

If you run "jj diff --tool meld", GUI diff will open and jj will wait for
meld to quit. This also applies to "jj log -p". The "diff --tool gui" behavior
is somewhat useful, but "log -p --tool gui" wouldn't. We might want some flag
to mark the tool output can't be streamed.

Another thing to consider is tools that can't generate directory diffs. Git
executes ext-diff tool per file, but we don't. Difftastic can compare
directories, and doing that should be more efficient since diffs can be
computed in parallel (at the expense of unsorted output.)

Closes jj-vcs#1886

CHANGELOG.md

@@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   from a merge revision's parents. This undoes the changes that `jj diff -r`
   would show.
 
+* `jj diff`/`log` now supports `--tool <name>` option to generate diffs by
+  external program. For configuration, see [the documentation](docs/config.md).
+  [#1886](https://github.com/martinvonz/jj/issues/1886)
 
 ### Fixed bugs
 

docs/config.md

@@ -141,6 +141,21 @@ ui.default-command = "log"
 ui.diff.format = "git"
 ```
 
+### Generating diffs by external command
+
+If `diff --tool <name>` argument is given, the external diff command will be
+called instead of the internal diff function. The command arguments can be
+specified as follows.
+
+```toml
+[merge-tools.<name>]
+# program = "<name>"  # Defaults to the name of the tool if not specified
+diff-args = ["--color=always", "$left", "$right"]
+```
+
+- `$left` and `$right` are replaced with the paths to the left and right
+  directories to diff respectively.
+
 ### Default revisions to log
 
 You can configure the revisions `jj log` without `-r` should show.

src/cli_util.rs

@@ -72,7 +72,7 @@ use crate::config::{
     new_config_path, AnnotatedValue, CommandNameAndArgs, ConfigSource, LayeredConfigs,
 };
 use crate::formatter::{FormatRecorder, Formatter, PlainTextFormatter};
-use crate::merge_tools::{ConflictResolveError, DiffEditError};
+use crate::merge_tools::{ConflictResolveError, DiffEditError, DiffGenerateError};
 use crate::template_parser::{TemplateAliasesMap, TemplateParseError};
 use crate::templater::Template;
 use crate::ui::{ColorChoice, Ui};
@@ -239,6 +239,12 @@ impl From<DiffEditError> for CommandError {
     }
 }
 
+impl From<DiffGenerateError> for CommandError {
+    fn from(err: DiffGenerateError) -> Self {
+        user_error(format!("Failed to generate diff: {err}"))
+    }
+}
+
 impl From<ConflictResolveError> for CommandError {
     fn from(err: ConflictResolveError) -> Self {
         user_error(format!("Failed to use external tool to resolve: {err}"))

src/config-schema.json

@@ -242,6 +242,12 @@
                     "program": {
                         "type": "string"
                     },
+                    "diff-args": {
+                        "type": "array",
+                        "items": {
+                            "type": "string"
+                        }
+                    },
                     "edit-args": {
                         "type": "array",
                         "items": {

src/diff_util.rs

@@ -32,10 +32,11 @@ use tracing::instrument;
 
 use crate::cli_util::{CommandError, WorkspaceCommandHelper};
 use crate::formatter::Formatter;
+use crate::merge_tools::{self, MergeTool};
 
 #[derive(clap::Args, Clone, Debug)]
 #[command(group(clap::ArgGroup::new("short-format").args(&["summary", "types"])))]
-#[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words"])))]
+#[command(group(clap::ArgGroup::new("long-format").args(&["git", "color_words", "tool"])))]
 pub struct DiffFormatArgs {
     /// For each path, show only whether it was modified, added, or removed
     #[arg(long, short)]
@@ -55,22 +56,26 @@ pub struct DiffFormatArgs {
     /// Show a word-level diff with changes indicated only by color
     #[arg(long)]
     pub color_words: bool,
+    /// Generate diff by external command
+    #[arg(long)]
+    pub tool: Option<String>,
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub enum DiffFormat {
     Summary,
     Types,
     Git,
     ColorWords,
+    Tool(Box<MergeTool>),
 }
 
 /// Returns a list of requested diff formats, which will never be empty.
 pub fn diff_formats_for(
     settings: &UserSettings,
     args: &DiffFormatArgs,
 ) -> Result<Vec<DiffFormat>, config::ConfigError> {
-    let formats = diff_formats_from_args(args);
+    let formats = diff_formats_from_args(settings, args)?;
     if formats.is_empty() {
         Ok(vec![default_diff_format(settings)?])
     } else {
@@ -85,7 +90,7 @@ pub fn diff_formats_for_log(
     args: &DiffFormatArgs,
     patch: bool,
 ) -> Result<Vec<DiffFormat>, config::ConfigError> {
-    let mut formats = diff_formats_from_args(args);
+    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)?);
@@ -94,16 +99,25 @@ pub fn diff_formats_for_log(
     Ok(formats)
 }
 
-fn diff_formats_from_args(args: &DiffFormatArgs) -> Vec<DiffFormat> {
-    [
+fn diff_formats_from_args(
+    settings: &UserSettings,
+    args: &DiffFormatArgs,
+) -> Result<Vec<DiffFormat>, config::ConfigError> {
+    let mut formats = [
         (args.summary, DiffFormat::Summary),
         (args.types, DiffFormat::Types),
         (args.git, DiffFormat::Git),
         (args.color_words, DiffFormat::ColorWords),
     ]
     .into_iter()
     .filter_map(|(arg, format)| arg.then_some(format))
-    .collect()
+    .collect_vec();
+    if let Some(name) = &args.tool {
+        let tool = merge_tools::get_tool_config(settings, name)?
+            .unwrap_or_else(|| MergeTool::with_program(name));
+        formats.push(DiffFormat::Tool(Box::new(tool)));
+    }
+    Ok(formats)
 }
 
 fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::ConfigError> {
@@ -120,6 +134,7 @@ fn default_diff_format(settings: &UserSettings) -> Result<DiffFormat, config::Co
         "types" => Ok(DiffFormat::Types),
         "git" => Ok(DiffFormat::Git),
         "color-words" => Ok(DiffFormat::ColorWords),
+        // TODO: add configurable default for DiffFormat::Tool?
         _ => Err(config::ConfigError::Message(format!(
             "invalid diff format: {name}"
         ))),
@@ -135,20 +150,26 @@ pub fn show_diff(
     formats: &[DiffFormat],
 ) -> Result<(), CommandError> {
     for format in formats {
-        let tree_diff = from_tree.diff(to_tree, matcher);
         match format {
             DiffFormat::Summary => {
+                let tree_diff = from_tree.diff(to_tree, matcher);
                 show_diff_summary(formatter, workspace_command, tree_diff)?;
             }
             DiffFormat::Types => {
+                let tree_diff = from_tree.diff(to_tree, matcher);
                 show_types(formatter, workspace_command, tree_diff)?;
             }
             DiffFormat::Git => {
+                let tree_diff = from_tree.diff(to_tree, matcher);
                 show_git_diff(formatter, workspace_command, tree_diff)?;
             }
             DiffFormat::ColorWords => {
+                let tree_diff = from_tree.diff(to_tree, matcher);
                 show_color_words_diff(formatter, workspace_command, tree_diff)?;
             }
+            DiffFormat::Tool(tool) => {
+                merge_tools::generate_diff(formatter.raw(), from_tree, to_tree, matcher, tool)?;
+            }
         }
     }
     Ok(())