From 03ea8779df47e5e03360129cb77f179221705549 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Mon, 31 May 2021 09:05:16 -0700
Subject: [PATCH] cli: make diff editor configurable

This patch adds a simple `ui.diff-editor` config, which defaults to
`meld`. This fixes issue #10.
---
 README.md        | 12 +++++++++---
 src/commands.rs  | 10 +++++-----
 src/diff_edit.rs | 12 +++++++++++-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index 248e87389..e04a10673 100644
--- a/README.md
+++ b/README.md
@@ -426,9 +426,15 @@ try `jj l --at-op=401652a2f61e` but use the hash from your own `jj op log`.
 
 You have already seen how `jj squash` can combine the changes from two commits
 into one. There are several other commands for changing the contents of existing
-commits. These commands require you to have `meld` installed for now
-(https://github.com/martinvonz/jj/issues/10). We'll need some more complex
-content to test these commands, so let's create a few more commits:
+commits. These commands assume that you have `meld` installed. If you prefer
+`vimdiff`, add this to your `~/.jjconfig` file:
+```
+[ui]
+diff-editor = "vimdiff"
+```
+
+We'll need some more complex content to test these commands, so let's create a
+few more commits:
 ```shell script
 $ jj co origin/main
 Working copy now at: 61b0efa09dbe 
diff --git a/src/commands.rs b/src/commands.rs
index 56edf5330..06081d562 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -1643,7 +1643,7 @@ fn cmd_squash(
             short_commit_description(&parent)
         );
         new_parent_tree_id =
-            crate::diff_edit::edit_diff(&parent.tree(), &commit.tree(), &instructions)?;
+            crate::diff_edit::edit_diff(ui, &parent.tree(), &commit.tree(), &instructions)?;
         if &new_parent_tree_id == parent.tree().id() {
             return Err(CommandError::UserError(String::from("No changes selected")));
         }
@@ -1702,7 +1702,7 @@ fn cmd_unsquash(
             short_commit_description(&commit)
         );
         new_parent_tree_id =
-            crate::diff_edit::edit_diff(&parent_base_tree, &parent.tree(), &instructions)?;
+            crate::diff_edit::edit_diff(ui, &parent_base_tree, &parent.tree(), &instructions)?;
         if &new_parent_tree_id == parent_base_tree.id() {
             return Err(CommandError::UserError(String::from("No changes selected")));
         }
@@ -1775,7 +1775,7 @@ fn cmd_restore(
             short_commit_description(&to_commit)
         );
         tree_id =
-            crate::diff_edit::edit_diff(&from_commit.tree(), &to_commit.tree(), &instructions)?;
+            crate::diff_edit::edit_diff(ui, &from_commit.tree(), &to_commit.tree(), &instructions)?;
     } else if sub_matches.is_present("paths") {
         let paths = sub_matches.values_of("paths").unwrap();
         let mut tree_builder = repo.store().tree_builder(to_commit.tree().id().clone());
@@ -1830,7 +1830,7 @@ fn cmd_edit(
         don't make any changes, then the operation will be aborted.\n",
         short_commit_description(&commit)
     );
-    let tree_id = crate::diff_edit::edit_diff(&base_tree, &commit.tree(), &instructions)?;
+    let tree_id = crate::diff_edit::edit_diff(ui, &base_tree, &commit.tree(), &instructions)?;
     if &tree_id == commit.tree().id() {
         ui.write("Nothing changed.\n")?;
     } else {
@@ -1867,7 +1867,7 @@ fn cmd_split(
         any changes, then the operation will be aborted.\n",
         short_commit_description(&commit)
     );
-    let tree_id = crate::diff_edit::edit_diff(&base_tree, &commit.tree(), &instructions)?;
+    let tree_id = crate::diff_edit::edit_diff(ui, &base_tree, &commit.tree(), &instructions)?;
     if &tree_id == commit.tree().id() {
         ui.write("Nothing changed.\n")?;
     } else {
diff --git a/src/diff_edit.rs b/src/diff_edit.rs
index d2a76f3e9..be91d8a3a 100644
--- a/src/diff_edit.rs
+++ b/src/diff_edit.rs
@@ -28,6 +28,8 @@ use jujutsu_lib::working_copy::{CheckoutError, TreeState};
 use tempfile::tempdir;
 use thiserror::Error;
 
+use crate::ui::Ui;
+
 #[derive(Debug, Error, PartialEq, Eq)]
 pub enum DiffEditError {
     #[error("The diff tool exited with a non-zero code")]
@@ -95,6 +97,7 @@ fn set_readonly_recursively(path: &Path) {
 }
 
 pub fn edit_diff(
+    ui: &mut Ui,
     left_tree: &Tree,
     right_tree: &Tree,
     instructions: &str,
@@ -145,8 +148,15 @@ pub fn edit_diff(
         file.write_all(instructions.as_bytes()).unwrap();
     }
 
+    // TODO: Make this configuration have a table of possible editors and detect the
+    // best one here.
+    let editor_binary = ui
+        .settings()
+        .config()
+        .get_str("ui.diff-editor")
+        .unwrap_or_else(|_| "meld".to_string());
     // Start a diff editor on the two directories.
-    let exit_status = Command::new("meld")
+    let exit_status = Command::new(&editor_binary)
         .arg(&left_wc_dir)
         .arg(&right_wc_dir)
         .status()