cli: add --tool=<name> option to diff/merge editing commands

CHANGELOG.md

@@ -75,6 +75,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj move --from/--to` can now be abbreviated to `jj move -f/-t`
 
+* `jj commit`/`diffedit`/`move`/`resolve`/`split`/`squash`/`unsquash` now accept
+  `--tool=<NAME>` option to override the default.
+   [#2575](https://github.com/martinvonz/jj/issues/2575)
+
 * Added completions for [Nushell](https://nushell.sh) to `jj util completion`
 
 * `jj branch list` now supports a `--tracked/-t` option which can be used to

cli/src/cli_util.rs

@@ -671,26 +671,50 @@ impl WorkspaceCommandHelper {
     }
 
     /// Loads diff editor from the settings.
-    // TODO: override settings by --tool= option (#2575)
-    pub fn diff_editor(&self, ui: &Ui) -> Result<DiffEditor, CommandError> {
+    ///
+    /// If the `tool_name` isn't specified, the default editor will be returned.
+    pub fn diff_editor(
+        &self,
+        ui: &Ui,
+        tool_name: Option<&str>,
+    ) -> Result<DiffEditor, CommandError> {
         let base_ignores = self.base_ignores()?;
-        Ok(DiffEditor::from_settings(ui, &self.settings, base_ignores)?)
+        if let Some(name) = tool_name {
+            Ok(DiffEditor::with_name(name, &self.settings, base_ignores)?)
+        } else {
+            Ok(DiffEditor::from_settings(ui, &self.settings, base_ignores)?)
+        }
     }
 
     /// Conditionally loads diff editor from the settings.
-    // TODO: override settings by --tool= option (#2575)
-    pub fn diff_selector(&self, ui: &Ui, interactive: bool) -> Result<DiffSelector, CommandError> {
-        if interactive {
-            Ok(DiffSelector::Interactive(self.diff_editor(ui)?))
+    ///
+    /// If the `tool_name` is specified, interactive session is implied.
+    pub fn diff_selector(
+        &self,
+        ui: &Ui,
+        tool_name: Option<&str>,
+        force_interactive: bool,
+    ) -> Result<DiffSelector, CommandError> {
+        if tool_name.is_some() || force_interactive {
+            Ok(DiffSelector::Interactive(self.diff_editor(ui, tool_name)?))
         } else {
             Ok(DiffSelector::NonInteractive)
         }
     }
 
     /// Loads 3-way merge editor from the settings.
-    // TODO: override settings by --tool= option (#2575)
-    pub fn merge_editor(&self, ui: &Ui) -> Result<MergeEditor, MergeToolConfigError> {
-        MergeEditor::from_settings(ui, &self.settings)
+    ///
+    /// If the `tool_name` isn't specified, the default editor will be returned.
+    pub fn merge_editor(
+        &self,
+        ui: &Ui,
+        tool_name: Option<&str>,
+    ) -> Result<MergeEditor, MergeToolConfigError> {
+        if let Some(name) = tool_name {
+            MergeEditor::with_name(name, &self.settings)
+        } else {
+            MergeEditor::from_settings(ui, &self.settings)
+        }
     }
 
     pub fn resolve_single_op(&self, op_str: &str) -> Result<Operation, OpsetEvaluationError> {

cli/src/commands/commit.rs

@@ -29,6 +29,9 @@ pub(crate) struct CommitArgs {
     /// Interactively choose which changes to include in the first commit
     #[arg(short, long)]
     interactive: bool,
+    /// Specify diff editor to be used (implies --interactive)
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
     /// The change description to use (don't open editor)
     #[arg(long = "message", short, value_name = "MESSAGE")]
     message_paragraphs: Vec<String>,
@@ -50,7 +53,8 @@ pub(crate) fn cmd_commit(
         .ok_or_else(|| user_error("This command requires a working copy"))?;
     let commit = workspace_command.repo().store().get_commit(commit_id)?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
+    let diff_selector =
+        workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
     let instructions = format!(

cli/src/commands/diffedit.rs

@@ -51,6 +51,9 @@ pub(crate) struct DiffeditArgs {
     /// Edit changes in this revision. Defaults to @ if --from is specified.
     #[arg(long, conflicts_with = "revision")]
     to: Option<RevisionArg>,
+    /// Specify diff editor to be used
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
 }
 
 #[instrument(skip_all)]
@@ -78,7 +81,7 @@ pub(crate) fn cmd_diffedit(
     };
     workspace_command.check_rewritable([&target_commit])?;
 
-    let diff_editor = workspace_command.diff_editor(ui)?;
+    let diff_editor = workspace_command.diff_editor(ui, args.tool.as_deref())?;
     let mut tx = workspace_command.start_transaction();
     let instructions = format!(
         "\

cli/src/commands/move.rs

@@ -50,8 +50,11 @@ pub(crate) struct MoveArgs {
     /// Interactively choose which parts to move
     #[arg(long, short)]
     interactive: bool,
+    /// Specify diff editor to be used (implies --interactive)
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
     /// Move only changes to these paths (instead of all paths)
-    #[arg(conflicts_with = "interactive", value_hint = clap::ValueHint::AnyPath)]
+    #[arg(conflicts_with_all = ["interactive", "tool"], value_hint = clap::ValueHint::AnyPath)]
     paths: Vec<String>,
 }
 
@@ -70,7 +73,8 @@ pub(crate) fn cmd_move(
     }
     workspace_command.check_rewritable([&source, &destination])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
+    let diff_selector =
+        workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?;
     let source_tree = source.tree()?;

cli/src/commands/resolve.rs

@@ -55,6 +55,9 @@ pub(crate) struct ResolveArgs {
     /// conflict
     #[arg(long, short, conflicts_with = "list")]
     quiet: bool,
+    /// Specify 3-way merge tool to be used
+    #[arg(long, conflicts_with = "list", value_name = "NAME")]
+    pub tool: Option<String>,
     /// Restrict to these paths when searching for a conflict to resolve. We
     /// will attempt to resolve the first conflict we can find. You can use
     /// the `--list` argument to find paths to use here.
@@ -97,7 +100,7 @@ pub(crate) fn cmd_resolve(
 
     let (repo_path, _) = conflicts.first().unwrap();
     workspace_command.check_rewritable([&commit])?;
-    let merge_editor = workspace_command.merge_editor(ui)?;
+    let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?;
     writeln!(
         ui.stderr(),
         "Resolving conflicts in: {}",

cli/src/commands/split.rs

@@ -41,6 +41,9 @@ pub(crate) struct SplitArgs {
     /// paths are provided.
     #[arg(long, short)]
     interactive: bool,
+    /// Specify diff editor to be used (implies --interactive)
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
     /// The revision to split
     #[arg(long, short, default_value = "@")]
     revision: RevisionArg,
@@ -59,8 +62,11 @@ pub(crate) fn cmd_split(
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
     workspace_command.check_rewritable([&commit])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let diff_selector =
-        workspace_command.diff_selector(ui, args.interactive || args.paths.is_empty())?;
+    let diff_selector = workspace_command.diff_selector(
+        ui,
+        args.tool.as_deref(),
+        args.interactive || args.paths.is_empty(),
+    )?;
     let mut tx = workspace_command.start_transaction();
     let end_tree = commit.tree()?;
     let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;

cli/src/commands/squash.rs

@@ -46,8 +46,11 @@ pub(crate) struct SquashArgs {
     /// Interactively choose which parts to squash
     #[arg(long, short)]
     interactive: bool,
+    /// Specify diff editor to be used (implies --interactive)
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
     /// Move only changes to these paths (instead of all paths)
-    #[arg(conflicts_with = "interactive", value_hint = clap::ValueHint::AnyPath)]
+    #[arg(conflicts_with_all = ["interactive", "tool"], value_hint = clap::ValueHint::AnyPath)]
     paths: Vec<String>,
 }
 
@@ -67,7 +70,8 @@ pub(crate) fn cmd_squash(
     let parent = &parents[0];
     workspace_command.check_rewritable(&parents[..1])?;
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let diff_selector = workspace_command.diff_selector(ui, args.interactive)?;
+    let diff_selector =
+        workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
     let instructions = format!(
         "\

cli/src/commands/unsquash.rs

@@ -45,6 +45,9 @@ pub(crate) struct UnsquashArgs {
     // the default.
     #[arg(long, short)]
     interactive: bool,
+    /// Specify diff editor to be used (implies --interactive)
+    #[arg(long, value_name = "NAME")]
+    pub tool: Option<String>,
 }
 
 #[instrument(skip_all)]
@@ -62,8 +65,8 @@ pub(crate) fn cmd_unsquash(
     }
     let parent = &parents[0];
     workspace_command.check_rewritable(&parents[..1])?;
-    let interactive_editor = if args.interactive {
-        Some(workspace_command.diff_editor(ui)?)
+    let interactive_editor = if args.tool.is_some() || args.interactive {
+        Some(workspace_command.diff_editor(ui, args.tool.as_deref())?)
     } else {
         None
     };