cli completion: complete --tool args for merge tools

Includes diff tools, diff editors, and merge editors.

cli/src/commands/commit.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use clap_complete::ArgValueCandidates;
 use clap_complete::ArgValueCompleter;
 use jj_lib::backend::Signature;
 use jj_lib::object_id::ObjectId as _;
@@ -35,7 +36,11 @@ pub(crate) struct CommitArgs {
     #[arg(short, long)]
     interactive: bool,
     /// Specify diff editor to be used (implies --interactive)
-    #[arg(long, value_name = "NAME")]
+    #[arg(
+        long,
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::diff_editors),
+    )]
     tool: Option<String>,
     /// The change description to use (don't open editor)
     #[arg(long = "message", short, value_name = "MESSAGE")]

cli/src/commands/diffedit.rs

@@ -78,7 +78,11 @@ pub(crate) struct DiffeditArgs {
     )]
     to: Option<RevisionArg>,
     /// Specify diff editor to be used
-    #[arg(long, value_name = "NAME")]
+    #[arg(
+        long,
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::diff_editors),
+    )]
     tool: Option<String>,
     /// Preserve the content (not the diff) when rebasing descendants
     ///

cli/src/commands/resolve.rs

@@ -62,7 +62,12 @@ pub(crate) struct ResolveArgs {
     ///
     /// The built-in merge tools `:ours` and `:theirs` can be used to choose
     /// side #1 and side #2 of the conflict respectively.
-    #[arg(long, conflicts_with = "list", value_name = "NAME")]
+    #[arg(
+        long,
+        conflicts_with = "list",
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::merge_tools),
+    )]
     tool: Option<String>,
     /// Only resolve conflicts in these paths. You can use the `--list` argument
     /// to find paths to use here.

cli/src/commands/restore.rs

@@ -97,7 +97,11 @@ pub(crate) struct RestoreArgs {
     #[arg(long, short)]
     interactive: bool,
     /// Specify diff editor to be used (implies --interactive)
-    #[arg(long, value_name = "NAME")]
+    #[arg(
+        long,
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::diff_editors),
+    )]
     tool: Option<String>,
     /// Preserve the content (not the diff) when rebasing descendants
     #[arg(long)]

cli/src/commands/split.rs

@@ -59,7 +59,11 @@ pub(crate) struct SplitArgs {
     #[arg(long, short)]
     interactive: bool,
     /// Specify diff editor to be used (implies --interactive)
-    #[arg(long, value_name = "NAME")]
+    #[arg(
+        long,
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::diff_editors),
+    )]
     tool: Option<String>,
     /// The revision to split
     #[arg(

cli/src/commands/squash.rs

@@ -99,7 +99,11 @@ pub(crate) struct SquashArgs {
     #[arg(long, short)]
     interactive: bool,
     /// Specify diff editor to be used (implies --interactive)
-    #[arg(long, value_name = "NAME")]
+    #[arg(
+        long,
+        value_name = "NAME",
+        add = ArgValueCandidates::new(complete::diff_editors),
+    )]
     tool: Option<String>,
     /// Move only changes to these paths (instead of all paths)
     #[arg(

cli/src/complete.rs

@@ -34,6 +34,8 @@ use crate::config::default_config_layers;
 use crate::config::ConfigArgKind;
 use crate::config::ConfigEnv;
 use crate::config::CONFIG_SCHEMA;
+use crate::merge_tools::configured_merge_tools;
+use crate::merge_tools::MergeEditor;
 use crate::revset_util::load_revset_aliases;
 use crate::ui::Ui;
 
@@ -412,6 +414,47 @@ pub fn workspaces() -> Vec<CompletionCandidate> {
             .collect())
     })
 }
+pub fn merge_tools() -> Vec<CompletionCandidate> {
+    with_jj(|_, settings| {
+        Ok([":builtin", ":ours", ":theirs"]
+            .into_iter()
+            .chain(
+                configured_merge_tools(settings)
+                    .filter(|name| MergeEditor::dummy_with_name(name, settings).is_ok()),
+            )
+            .map(CompletionCandidate::new)
+            .collect())
+    })
+}
+
+/// Approximate list of known diff editors
+///
+/// Diff tools can be used without configuration. Some merge tools that are
+/// configured for 3-way merging may not work for diffing/diff editing, and we
+/// can't tell which these are. So, this not reliable, but probably good enough
+/// for command-line completion.
+pub fn diff_editors() -> Vec<CompletionCandidate> {
+    with_jj(|_, settings| {
+        Ok(std::iter::once(":builtin")
+            .chain(configured_merge_tools(settings))
+            .map(CompletionCandidate::new)
+            .collect())
+    })
+}
+
+/// Approximate list of known diff tools
+///
+/// Diff tools can be used without configuration. Some merge tools that are
+/// configured for 3-way merging may not work for diffing/diff editing, and we
+/// can't tell which these are. So, this not reliable, but probably good enough
+/// for command-line completion.
+pub fn diff_tools() -> Vec<CompletionCandidate> {
+    with_jj(|_, settings| {
+        Ok(configured_merge_tools(settings)
+            .map(CompletionCandidate::new)
+            .collect())
+    })
+}
 
 fn config_keys_rec(
     prefix: ConfigNamePathBuf,

cli/src/diff_util.rs

@@ -23,6 +23,7 @@ use std::path::PathBuf;
 
 use bstr::BStr;
 use bstr::BString;
+use clap_complete::ArgValueCandidates;
 use futures::executor::block_on_stream;
 use futures::stream::BoxStream;
 use futures::StreamExt as _;
@@ -117,7 +118,10 @@ pub struct DiffFormatArgs {
     #[arg(long)]
     pub color_words: bool,
     /// Generate diff by external command
-    #[arg(long)]
+    #[arg(
+        long,
+        add = ArgValueCandidates::new(crate::complete::diff_tools),
+    )]
     pub tool: Option<String>,
     /// Number of lines of context to show
     #[arg(long)]

cli/src/merge_tools/mod.rs

@@ -204,6 +204,11 @@ fn editor_args_from_settings(
     }
 }
 
+/// List configured merge tools (diff editors, diff tools, merge editors)
+pub fn configured_merge_tools(settings: &UserSettings) -> impl Iterator<Item = &str> {
+    settings.table_keys("merge-tools")
+}
+
 /// Loads external diff/merge tool options from `[merge-tools.<name>]`.
 pub fn get_external_tool_config(
     settings: &UserSettings,
@@ -376,6 +381,22 @@ impl MergeEditor {
         Self::new_inner(name, tool, path_converter, conflict_marker_style)
     }
 
+    /// For the purposes of testing or checking basic config
+    pub fn dummy_with_name(
+        name: &str,
+        settings: &UserSettings,
+    ) -> Result<Self, MergeToolConfigError> {
+        Self::with_name(
+            name,
+            settings,
+            RepoPathUiConverter::Fs {
+                cwd: "".into(),
+                base: "".into(),
+            },
+            ConflictMarkerStyle::Diff,
+        )
+    }
+
     /// Loads the default 3-way merge editor from the settings.
     pub fn from_settings(
         ui: &Ui,
@@ -762,12 +783,7 @@ mod tests {
         let get = |name, config_text| {
             let config = config_from_string(config_text);
             let settings = UserSettings::from_config(config).unwrap();
-            let path_converter = RepoPathUiConverter::Fs {
+            MergeEditor::dummy_with_name(name, &settings).map(|editor| editor.tool)
-                cwd: "".into(),
-                base: "".into(),
-            };
-            MergeEditor::with_name(name, &settings, path_converter, ConflictMarkerStyle::Diff)
-                .map(|editor| editor.tool)
         };
 
         insta::assert_debug_snapshot!(get(":builtin", "").unwrap(), @"Builtin");

cli/tests/test_completion.rs

@@ -772,6 +772,61 @@ fn test_template_alias() {
     ");
 }
 
+#[test]
+fn test_merge_tools() {
+    let mut test_env = TestEnvironment::default();
+    test_env.add_env_var("COMPLETE", "fish");
+    let dir = test_env.env_root();
+
+    let output = test_env.run_jj_in(dir, ["--", "jj", "diff", "--tool", ""]);
+    insta::assert_snapshot!(output, @r"
+    diffedit3
+    diffedit3-ssh
+    difft
+    kdiff3
+    meld
+    meld-3
+    mergiraf
+    smerge
+    vimdiff
+    vscode
+    vscodium
+    [EOF]
+    ");
+    // Includes :builtin
+    let output = test_env.run_jj_in(dir, ["--", "jj", "diffedit", "--tool", ""]);
+    insta::assert_snapshot!(output, @r"
+    :builtin
+    diffedit3
+    diffedit3-ssh
+    difft
+    kdiff3
+    meld
+    meld-3
+    mergiraf
+    smerge
+    vimdiff
+    vscode
+    vscodium
+    [EOF]
+    ");
+    // Only includes configured merge editors
+    let output = test_env.run_jj_in(dir, ["--", "jj", "resolve", "--tool", ""]);
+    insta::assert_snapshot!(output, @r"
+    :builtin
+    :ours
+    :theirs
+    kdiff3
+    meld
+    mergiraf
+    smerge
+    vimdiff
+    vscode
+    vscodium
+    [EOF]
+    ");
+}
+
 fn create_commit(
     work_dir: &TestWorkDir,
     name: &str,