From 8e727de2ab3589d1cd5d9530c0809d9793165edd Mon Sep 17 00:00:00 2001
From: Lukas Wirth <lukastw97@gmail.com>
Date: Thu, 12 Sep 2024 16:20:57 +0200
Subject: [PATCH] undo: Report what operation has been undone in `jj op undo`

---
 CHANGELOG.md                        |  2 ++
 cli/src/commands/operation/undo.rs  | 22 ++++++++++++++++++++++
 cli/src/config/templates.toml       |  7 +++++++
 cli/tests/test_duplicate_command.rs |  8 ++++++--
 cli/tests/test_evolog_command.rs    |  4 ++--
 cli/tests/test_git_colocated.rs     |  5 +++--
 cli/tests/test_git_fetch.rs         |  4 +++-
 cli/tests/test_git_import_export.rs |  4 +++-
 cli/tests/test_log_command.rs       |  4 ++--
 cli/tests/test_operations.rs        | 20 ++++++++++----------
 cli/tests/test_show_command.rs      |  4 ++--
 cli/tests/test_templater.rs         |  4 ++--
 12 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1ef1952375..f753571657 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -56,6 +56,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
   allows to set a name for the remote instead of using the default
   `origin`.
 
+* `jj op undo` now reports information on the operation that has been undone.
+
 ### Fixed bugs
 
  * Fixed panic when parsing invalid conflict markers of a particular form.
diff --git a/cli/src/commands/operation/undo.rs b/cli/src/commands/operation/undo.rs
index 22bba29e47..d4e7900803 100644
--- a/cli/src/commands/operation/undo.rs
+++ b/cli/src/commands/operation/undo.rs
@@ -21,6 +21,7 @@ use super::DEFAULT_UNDO_WHAT;
 use crate::cli_util::CommandHelper;
 use crate::command_error::user_error;
 use crate::command_error::CommandError;
+use crate::operation_templater::OperationTemplateLanguage;
 use crate::ui::Ui;
 
 /// Create a new operation that undoes an earlier operation
@@ -70,5 +71,26 @@ pub fn cmd_op_undo(
     tx.repo_mut().set_view(new_view);
     tx.finish(ui, format!("undo operation {}", bad_op.id().hex()))?;
 
+    if let Some(mut formatter) = ui.status_formatter() {
+        write!(formatter, "Undid operation ")?;
+        let workspace_env = workspace_command.env();
+        let language = OperationTemplateLanguage::new(
+            bad_op.op_store().root_operation_id(),
+            Some(bad_op.id()),
+            workspace_env.operation_template_extensions(),
+        );
+        let text = command
+            .settings()
+            .config()
+            .get_string("templates.op_summary")?;
+        let template = workspace_env.parse_template(
+            &language,
+            &text,
+            OperationTemplateLanguage::wrap_operation,
+        )?;
+        template.format(&bad_op, &mut *formatter)?;
+        writeln!(formatter)?;
+    }
+
     Ok(())
 }
diff --git a/cli/src/config/templates.toml b/cli/src/config/templates.toml
index 6fd6b7c268..77c6995ea8 100644
--- a/cli/src/config/templates.toml
+++ b/cli/src/config/templates.toml
@@ -41,6 +41,13 @@ tag_list = '''
 label("tag", name) ++ format_ref_targets(self) ++ "\n"
 '''
 
+op_summary = '''
+if(root,
+  separate(" ", self.id().short(), label("root", "root()")),
+  separate(" ", self.id().short(), format_time_range(self.time()), self.description().first_line())
+)
+'''
+
 [template-aliases]
 builtin_log_oneline = '''
 if(root,
diff --git a/cli/tests/test_duplicate_command.rs b/cli/tests/test_duplicate_command.rs
index c9f0b23db5..62c57bd26d 100644
--- a/cli/tests/test_duplicate_command.rs
+++ b/cli/tests/test_duplicate_command.rs
@@ -75,7 +75,9 @@ fn test_duplicate() {
 
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["undo"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Undid operation c4b0b2a977fe 2001-02-03 04:05:17.000 +07:00 - 2001-02-03 04:05:17.000 +07:00 duplicate 1 commit(s)
+    "#);
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate" /* duplicates `c` */]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
@@ -277,7 +279,9 @@ fn test_undo_after_duplicate() {
 
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["undo"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Undid operation 5a2bcfcdd78b 2001-02-03 04:05:11.000 +07:00 - 2001-02-03 04:05:11.000 +07:00 duplicate 1 commit(s)
+    "#);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  2443ea76b0b1   a
     ◆  000000000000
diff --git a/cli/tests/test_evolog_command.rs b/cli/tests/test_evolog_command.rs
index f61b468692..4fc2b949e6 100644
--- a/cli/tests/test_evolog_command.rs
+++ b/cli/tests/test_evolog_command.rs
@@ -327,7 +327,7 @@ fn test_evolog_with_no_template() {
     let repo_path = test_env.env_root().join("repo");
 
     let stderr = test_env.jj_cmd_cli_error(&repo_path, &["evolog", "-T"]);
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     error: a value is required for '--template <TEMPLATE>' but none was supplied
 
     For more information, try '--help'.
@@ -346,5 +346,5 @@ fn test_evolog_with_no_template() {
     - description_placeholder
     - email_placeholder
     - name_placeholder
-    "###);
+    "#);
 }
diff --git a/cli/tests/test_git_colocated.rs b/cli/tests/test_git_colocated.rs
index 37885ad39e..41b270a435 100644
--- a/cli/tests/test_git_colocated.rs
+++ b/cli/tests/test_git_colocated.rs
@@ -756,10 +756,11 @@ fn test_git_colocated_undo_head_move() {
     // HEAD should be moved back
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["undo"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     Working copy now at: royxmykx eb08b363 (empty) (no description set)
     Parent commit      : qpvuntsm 230dd059 (empty) (no description set)
-    "###);
+    Undid operation 951a1c249028 2001-02-03 04:05:13.000 +07:00 - 2001-02-03 04:05:13.000 +07:00 new empty commit
+    "#);
     insta::assert_snapshot!(
         git_repo.head().unwrap().target().unwrap().to_string(),
         @"230dd059e1b059aefc0da06a2e5a7dbf22362f22");
diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs
index 977ee63ea7..6311444c95 100644
--- a/cli/tests/test_git_fetch.rs
+++ b/cli/tests/test_git_fetch.rs
@@ -845,7 +845,9 @@ fn test_git_fetch_undo() {
     "###);
     let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["undo"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Undid operation bbebbf6a6ebe 2001-02-03 04:05:18.000 +07:00 - 2001-02-03 04:05:18.000 +07:00 fetch from git remote(s) origin
+    "#);
     // The undo works as expected
     insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
     @  230dd059e1b0
diff --git a/cli/tests/test_git_import_export.rs b/cli/tests/test_git_import_export.rs
index 72f29bb5da..c92c41cd56 100644
--- a/cli/tests/test_git_import_export.rs
+++ b/cli/tests/test_git_import_export.rs
@@ -100,7 +100,9 @@ fn test_git_export_undo() {
     // bookmark is. This is the same as remote-tracking bookmarks.
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "undo"]);
     insta::assert_snapshot!(stdout, @"");
-    insta::assert_snapshot!(stderr, @"");
+    insta::assert_snapshot!(stderr, @r#"
+    Undid operation 1bc51ad79d63 2001-02-03 04:05:10.000 +07:00 - 2001-02-03 04:05:10.000 +07:00 export git refs
+    "#);
     insta::assert_debug_snapshot!(get_git_repo_refs(&git_repo), @r###"
     [
         (
diff --git a/cli/tests/test_log_command.rs b/cli/tests/test_log_command.rs
index 60555832a5..7d59506f41 100644
--- a/cli/tests/test_log_command.rs
+++ b/cli/tests/test_log_command.rs
@@ -36,7 +36,7 @@ fn test_log_with_no_template() {
     let repo_path = test_env.env_root().join("repo");
 
     let stderr = test_env.jj_cmd_cli_error(&repo_path, &["log", "-T"]);
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     error: a value is required for '--template <TEMPLATE>' but none was supplied
 
     For more information, try '--help'.
@@ -55,7 +55,7 @@ fn test_log_with_no_template() {
     - description_placeholder
     - email_placeholder
     - name_placeholder
-    "###);
+    "#);
 }
 
 #[test]
diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs
index 5c2c7adddf..de06bf1929 100644
--- a/cli/tests/test_operations.rs
+++ b/cli/tests/test_operations.rs
@@ -159,7 +159,7 @@ fn test_op_log_with_no_template() {
     let repo_path = test_env.env_root().join("repo");
 
     let stderr = test_env.jj_cmd_cli_error(&repo_path, &["op", "log", "-T"]);
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     error: a value is required for '--template <TEMPLATE>' but none was supplied
 
     For more information, try '--help'.
@@ -178,7 +178,7 @@ fn test_op_log_with_no_template() {
     - description_placeholder
     - email_placeholder
     - name_placeholder
-    "###);
+    "#);
 }
 
 #[test]
@@ -871,10 +871,10 @@ fn test_op_diff() {
 
     "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["op", "diff", "--from", "@", "--to", "@"]);
-    insta::assert_snapshot!(&stdout, @r###"
-    From operation ea112f6a02be: check out git remote's default branch
-      To operation ea112f6a02be: check out git remote's default branch
-    "###);
+    insta::assert_snapshot!(&stdout, @r#"
+    From operation ea112f6a02be 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 check out git remote's default branch
+      To operation ea112f6a02be 2001-02-03 04:05:07.000 +07:00 - 2001-02-03 04:05:07.000 +07:00 check out git remote's default branch
+    "#);
 
     // Diff from parent operation to latest operation.
     // `jj op diff --op @` should behave identically to `jj op diff --from
@@ -1312,9 +1312,9 @@ fn test_op_diff_patch() {
     Parent commit      : qpvuntsm 2ac85fd1 (no description set)
     "###);
     let stdout = test_env.jj_cmd_success(&repo_path, &["op", "diff", "-p", "--git"]);
-    insta::assert_snapshot!(&stdout, @r###"
-    From operation 874d3a8b4c77: snapshot working copy
-      To operation c53f5f1afbc6: squash commits into 6b1027d2770cd0a39c468e525e52bf8c47e1464a
+    insta::assert_snapshot!(&stdout, @r#"
+    From operation 874d3a8b4c77 2001-02-03 04:05:11.000 +07:00 - 2001-02-03 04:05:11.000 +07:00 snapshot working copy
+      To operation c53f5f1afbc6 2001-02-03 04:05:11.000 +07:00 - 2001-02-03 04:05:11.000 +07:00 squash commits into 6b1027d2770cd0a39c468e525e52bf8c47e1464a
 
     Changed commits:
     ○  Change mzvwutvlkqwt
@@ -1338,7 +1338,7 @@ fn test_op_diff_patch() {
        @@ -1,1 +1,1 @@
        -a
        +b
-    "###);
+    "#);
 
     // Abandon the working copy commit.
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon"]);
diff --git a/cli/tests/test_show_command.rs b/cli/tests/test_show_command.rs
index 7ae53abb6d..614d077ad6 100644
--- a/cli/tests/test_show_command.rs
+++ b/cli/tests/test_show_command.rs
@@ -250,7 +250,7 @@ fn test_show_with_no_template() {
     let repo_path = test_env.env_root().join("repo");
 
     let stderr = test_env.jj_cmd_cli_error(&repo_path, &["show", "-T"]);
-    insta::assert_snapshot!(stderr, @r###"
+    insta::assert_snapshot!(stderr, @r#"
     error: a value is required for '--template <TEMPLATE>' but none was supplied
 
     For more information, try '--help'.
@@ -269,7 +269,7 @@ fn test_show_with_no_template() {
     - description_placeholder
     - email_placeholder
     - name_placeholder
-    "###);
+    "#);
 }
 
 #[test]
diff --git a/cli/tests/test_templater.rs b/cli/tests/test_templater.rs
index 65484e0b0b..dd72d260a9 100644
--- a/cli/tests/test_templater.rs
+++ b/cli/tests/test_templater.rs
@@ -103,7 +103,7 @@ fn test_templater_parse_error() {
 
     // -Tbuiltin shows the predefined builtin_* aliases. This isn't 100%
     // guaranteed, but is nice.
-    insta::assert_snapshot!(render_err(r#"builtin"#), @r###"
+    insta::assert_snapshot!(render_err(r#"builtin"#), @r#"
     Error: Failed to parse template: Keyword "builtin" doesn't exist
     Caused by:  --> 1:1
       |
@@ -112,7 +112,7 @@ fn test_templater_parse_error() {
       |
       = Keyword "builtin" doesn't exist
     Hint: Did you mean "builtin_log_comfortable", "builtin_log_compact", "builtin_log_detailed", "builtin_log_node", "builtin_log_node_ascii", "builtin_log_oneline", "builtin_op_log_comfortable", "builtin_op_log_compact", "builtin_op_log_node", "builtin_op_log_node_ascii"?
-    "###);
+    "#);
 }
 
 #[test]