diff: add a file-by-file variant for external diff tools

CHANGELOG.md

@@ -72,6 +72,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * New command `jj git remote set-url` that sets the url of a git remote.
 
+* External diff tools now support `--tool-mode=file-by-file` which invokes the
+  tool on individual files instead of a directory.
+
 ### Fixed bugs
 
 * `jj git push` now ignores immutable commits when checking whether a

cli/src/config-schema.json

@@ -112,6 +112,14 @@
                         "tool": {
                             "type": "string",
                             "description": "External tool for generating diffs"
+                        },
+                        "tool-mode": {
+                            "description": "Invoke the external tool with directories or individual files",
+                            "enum": [
+                                "dir",
+                                "file-by-file"
+                            ],
+                            "default": "dir"
                         }
                     }
                 },

cli/src/diff_util.rs

@@ -14,8 +14,11 @@
 
 use std::cmp::max;
 use std::collections::VecDeque;
+use std::fs::File;
 use std::io;
+use std::io::Write;
 use std::ops::Range;
+use std::path::{Path, PathBuf};
 
 use futures::{try_join, Stream, StreamExt};
 use itertools::Itertools;
@@ -40,7 +43,10 @@ use unicode_width::UnicodeWidthStr as _;
 
 use crate::config::CommandNameAndArgs;
 use crate::formatter::Formatter;
-use crate::merge_tools::{self, DiffGenerateError, ExternalMergeTool};
+use crate::merge_tools::{
+    self, generate_diff, invoke_external_diff, new_utf8_temp_dir, DiffGenerateError,
+    ExternalMergeTool,
+};
 use crate::text_util;
 use crate::ui::Ui;
 
@@ -81,20 +87,38 @@ pub struct DiffFormatArgs {
     /// Generate diff by external command
     #[arg(long)]
     pub tool: Option<String>,
+    #[arg(long, requires("tool"), default_value("dir"))]
+    pub tool_mode: DiffToolMode,
     /// Number of lines of context to show
     #[arg(long)]
     context: Option<usize>,
 }
 
+#[derive(clap::ValueEnum, serde::Deserialize, Copy, Clone, Debug, Eq, PartialEq)]
+#[serde(rename_all = "kebab-case")]
+pub enum DiffToolMode {
+    /// Invoke the diff tool on a temp directory of the modified files.
+    Dir,
+    /// Invoke the diff tool on each of the modified files individually.
+    FileByFile,
+}
+
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum DiffFormat {
     Summary,
     Stat,
     Types,
     NameOnly,
-    Git { context: usize },
-    ColorWords { context: usize },
-    Tool(Box<ExternalMergeTool>),
+    Git {
+        context: usize,
+    },
+    ColorWords {
+        context: usize,
+    },
+    Tool {
+        tool: Box<ExternalMergeTool>,
+        mode: DiffToolMode,
+    },
 }
 
 /// Returns a list of requested diff formats, which will never be empty.
@@ -154,7 +178,10 @@ fn diff_formats_from_args(
     if let Some(name) = &args.tool {
         let tool = merge_tools::get_external_tool_config(settings, name)?
             .unwrap_or_else(|| ExternalMergeTool::with_program(name));
-        formats.push(DiffFormat::Tool(Box::new(tool)));
+        formats.push(DiffFormat::Tool {
+            tool: Box::new(tool),
+            mode: args.tool_mode,
+        });
     }
     Ok(formats)
 }
@@ -172,7 +199,14 @@ fn default_diff_format(
             None
         }
         .unwrap_or_else(|| ExternalMergeTool::with_diff_args(&args));
-        return Ok(DiffFormat::Tool(Box::new(tool)));
+        let mode = config
+            .get("ui.diff.tool-mode")
+            .optional()?
+            .unwrap_or(DiffToolMode::Dir);
+        return Ok(DiffFormat::Tool {
+            tool: Box::new(tool),
+            mode,
+        });
     }
     let name = if let Some(name) = config.get_string("ui.diff.format").optional()? {
         name
@@ -272,16 +306,19 @@ impl<'a> DiffRenderer<'a> {
                     let tree_diff = from_tree.diff_stream(to_tree, matcher);
                     show_color_words_diff(repo, formatter, *context, tree_diff, path_converter)?;
                 }
-                DiffFormat::Tool(tool) => {
-                    merge_tools::generate_diff(
-                        ui,
-                        formatter.raw(),
-                        from_tree,
-                        to_tree,
-                        matcher,
-                        tool,
-                    )
-                    .map_err(DiffRenderError::DiffGenerate)?;
+                DiffFormat::Tool {
+                    tool,
+                    mode: DiffToolMode::FileByFile,
+                } => {
+                    let tree_diff = from_tree.diff_stream(to_tree, matcher);
+                    show_file_by_file_diff(ui, repo, formatter, tool, tree_diff, path_converter)?;
+                }
+                DiffFormat::Tool {
+                    tool,
+                    mode: DiffToolMode::Dir,
+                } => {
+                    generate_diff(ui, formatter.raw(), from_tree, to_tree, matcher, tool)
+                        .map_err(DiffRenderError::DiffGenerate)?;
                 }
             }
         }
@@ -644,6 +681,67 @@ pub fn show_color_words_diff(
     Ok(())
 }
 
+pub fn show_file_by_file_diff(
+    ui: &Ui,
+    repo: &dyn Repo,
+    formatter: &mut dyn Formatter,
+    tool: &ExternalMergeTool,
+    tree_diff: TreeDiffStream,
+    path_converter: &RepoPathUiConverter,
+) -> Result<(), DiffRenderError> {
+    fn create_file(
+        path: &RepoPath,
+        wc_dir: &Path,
+        value: MaterializedTreeValue,
+    ) -> Result<PathBuf, DiffRenderError> {
+        let fs_path = path.to_fs_path(wc_dir);
+        std::fs::create_dir_all(fs_path.parent().unwrap())?;
+        let content = diff_content(path, value)?;
+        let mut buffer = File::create(&fs_path)?;
+        buffer.write_all(&content.contents)?;
+        Ok(fs_path)
+    }
+
+    let temp_dir = new_utf8_temp_dir("jj-diff-")?;
+    let left_wc_dir = temp_dir.path().join("left");
+    let right_wc_dir = temp_dir.path().join("right");
+    let mut diff_stream = materialized_diff_stream(repo.store(), tree_diff);
+    async {
+        while let Some((path, diff)) = diff_stream.next().await {
+            let ui_path = path_converter.format_file_path(&path);
+            let (left_value, right_value) = diff?;
+
+            match (&left_value, &right_value) {
+                (_, MaterializedTreeValue::AccessDenied(source))
+                | (MaterializedTreeValue::AccessDenied(source), _) => {
+                    write!(
+                        formatter.labeled("access-denied"),
+                        "Access denied to {ui_path}:"
+                    )?;
+                    writeln!(formatter, " {source}")?;
+                    continue;
+                }
+                _ => {}
+            }
+            let left_path = create_file(&path, &left_wc_dir, left_value)?;
+            let right_path = create_file(&path, &right_wc_dir, right_value)?;
+
+            invoke_external_diff(
+                ui,
+                formatter.raw(),
+                tool,
+                maplit::hashmap! {
+                    "left" => left_path.to_str().expect("temp_dir should be valid utf-8"),
+                    "right" => right_path.to_str().expect("temp_dir should be valid utf-8"),
+                },
+            )
+            .map_err(DiffRenderError::DiffGenerate)?;
+        }
+        Ok::<(), DiffRenderError>(())
+    }
+    .block_on()
+}
+
 struct GitDiffPart {
     mode: String,
     hash: String,

cli/src/merge_tools/external.rs

@@ -257,7 +257,7 @@ pub fn edit_diff_external(
     diffedit_wc.snapshot_results(base_ignores)
 }
 
-/// Generates textual diff by the specified `tool`, and writes into `writer`.
+/// Generates textual diff by the specified `tool` and writes into `writer`.
 pub fn generate_diff(
     ui: &Ui,
     writer: &mut dyn Write,
@@ -272,9 +272,17 @@ pub fn generate_diff(
         .map_err(ExternalToolError::SetUpDir)?;
     set_readonly_recursively(diff_wc.right_working_copy_path())
         .map_err(ExternalToolError::SetUpDir)?;
-    // TODO: Add support for tools without directory diff functionality?
+    invoke_external_diff(ui, writer, tool, diff_wc.to_command_variables())
+}
+
+/// Invokes the specified `tool` directing its output into `writer`.
+pub fn invoke_external_diff(
+    ui: &Ui,
+    writer: &mut dyn Write,
+    tool: &ExternalMergeTool,
+    patterns: HashMap<&str, &str>,
+) -> Result<(), DiffGenerateError> {
     // TODO: Somehow propagate --color to the external command?
-    let patterns = diff_wc.to_command_variables();
     let mut cmd = Command::new(&tool.program);
     cmd.args(interpolate_variables(&tool.diff_args, &patterns));
     tracing::info!(?cmd, "Invoking the external diff generator:");

cli/src/merge_tools/mod.rs

@@ -31,9 +31,10 @@ use pollster::FutureExt;
 use thiserror::Error;
 
 use self::builtin::{edit_diff_builtin, edit_merge_builtin, BuiltinToolError};
+pub(crate) use self::diff_working_copies::new_utf8_temp_dir;
 use self::diff_working_copies::DiffCheckoutError;
 use self::external::{edit_diff_external, ExternalToolError};
-pub use self::external::{generate_diff, ExternalMergeTool};
+pub use self::external::{generate_diff, invoke_external_diff, ExternalMergeTool};
 use crate::config::CommandNameAndArgs;
 use crate::ui::Ui;
 

cli/testing/fake-diff-editor.rs

@@ -34,17 +34,21 @@ struct Args {
     _ignore: Vec<String>,
 }
 
-fn files_recursively(dir: &Path) -> HashSet<String> {
+fn files_recursively(p: &Path) -> HashSet<String> {
     let mut files = HashSet::new();
-    for dir_entry in std::fs::read_dir(dir).unwrap() {
-        let dir_entry = dir_entry.unwrap();
-        let base_name = dir_entry.file_name().to_str().unwrap().to_string();
-        if dir_entry.path().is_dir() {
-            for sub_path in files_recursively(&dir_entry.path()) {
-                files.insert(format!("{base_name}/{sub_path}"));
+    if !p.is_dir() {
+        files.insert(p.file_name().unwrap().to_str().unwrap().to_string());
+    } else {
+        for dir_entry in std::fs::read_dir(p).unwrap() {
+            let dir_entry = dir_entry.unwrap();
+            let base_name = dir_entry.file_name().to_str().unwrap().to_string();
+            if !dir_entry.path().is_dir() {
+                files.insert(base_name);
+            } else {
+                for sub_path in files_recursively(&dir_entry.path()) {
+                    files.insert(format!("{base_name}/{sub_path}"));
+                }
             }
-        } else {
-            files.insert(base_name);
         }
     }
     files

cli/tests/cli-reference@.md.snap

@@ -621,6 +621,16 @@ With the `--from` and/or `--to` options, shows the difference from/to the given
 * `--git` — Show a Git-format diff
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
+* `--tool-mode <TOOL_MODE>`
+
+  Default value: `dir`
+
+  Possible values:
+  - `dir`:
+    Invoke the diff tool on a temp directory of the modified files
+  - `file-by-file`:
+    Invoke the diff tool on each of the modified files individually
+
 * `--context <CONTEXT>` — Number of lines of context to show
 
 
@@ -1062,6 +1072,16 @@ This excludes changes from other commits by temporarily rebasing `--from` onto `
 * `--git` — Show a Git-format diff
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
+* `--tool-mode <TOOL_MODE>`
+
+  Default value: `dir`
+
+  Possible values:
+  - `dir`:
+    Invoke the diff tool on a temp directory of the modified files
+  - `file-by-file`:
+    Invoke the diff tool on each of the modified files individually
+
 * `--context <CONTEXT>` — Number of lines of context to show
 
 
@@ -1103,6 +1123,16 @@ Spans of revisions that are not included in the graph per `--revisions` are rend
 * `--git` — Show a Git-format diff
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
+* `--tool-mode <TOOL_MODE>`
+
+  Default value: `dir`
+
+  Possible values:
+  - `dir`:
+    Invoke the diff tool on a temp directory of the modified files
+  - `file-by-file`:
+    Invoke the diff tool on each of the modified files individually
+
 * `--context <CONTEXT>` — Number of lines of context to show
 
 
@@ -1216,6 +1246,16 @@ Name is derived from Merciual's obsolescence markers.
 * `--git` — Show a Git-format diff
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
+* `--tool-mode <TOOL_MODE>`
+
+  Default value: `dir`
+
+  Possible values:
+  - `dir`:
+    Invoke the diff tool on a temp directory of the modified files
+  - `file-by-file`:
+    Invoke the diff tool on each of the modified files individually
+
 * `--context <CONTEXT>` — Number of lines of context to show
 
 
@@ -1627,6 +1667,16 @@ Show commit description and changes in a revision
 * `--git` — Show a Git-format diff
 * `--color-words` — Show a word-level diff with changes indicated only by color
 * `--tool <TOOL>` — Generate diff by external command
+* `--tool-mode <TOOL_MODE>`
+
+  Default value: `dir`
+
+  Possible values:
+  - `dir`:
+    Invoke the diff tool on a temp directory of the modified files
+  - `file-by-file`:
+    Invoke the diff tool on each of the modified files individually
+
 * `--context <CONTEXT>` — Number of lines of context to show