From 87ba11592b064cbdab3adff2a38ad60420a03352 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 12 May 2022 22:20:10 -0700
Subject: [PATCH] cli: make `log -s/--git` imply `-p`

`log -s/--summary` and `log --git` without `-p` don't do anything. I
also don't think it's very useful to pass these flags in an alias,
where you would then sometimes also pass `-p` to see the diff summary
in the output. We already have the `diff.format` config for that use
case. So let's make both of these flags imply `-p`.

I implemented it by making the `diff_format` variable an
`Option<DiffFormat>`, which is set iff we should show a patch. That
way we have the condition in one place, and the places we use it
cannot forget to check it.
---
 src/commands.rs           | 11 ++++-----
 tests/test_log_command.rs | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/src/commands.rs b/src/commands.rs
index 0c1bbe378..f75e68d45 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -1287,7 +1287,7 @@ struct LogArgs {
     #[clap(long, short = 'p')]
     patch: bool,
     #[clap(flatten)]
-    format: DiffFormatArgs,
+    diff_format: DiffFormatArgs,
 }
 
 /// Show how a change has evolved
@@ -2993,7 +2993,8 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
     let checkout_id = repo.view().get_checkout(&workspace_id);
     let revset = revset_expression.evaluate(repo.as_repo_ref(), Some(&workspace_id))?;
     let store = repo.store();
-    let diff_format = diff_format_for(ui, &args.format);
+    let diff_format = (args.patch || args.diff_format.git || args.diff_format.summary)
+        .then(|| diff_format_for(ui, &args.diff_format));
 
     let template_string = match &args.template {
         Some(value) => value.to_string(),
@@ -3053,7 +3054,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
             if !buffer.ends_with(b"\n") {
                 buffer.push(b'\n');
             }
-            if args.patch {
+            if let Some(diff_format) = diff_format {
                 let writer = Box::new(&mut buffer);
                 let mut formatter = ui.new_formatter(writer);
                 show_patch(formatter.as_mut(), &workspace_command, &commit, diff_format)?;
@@ -3070,9 +3071,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
         for index_entry in revset.iter() {
             let commit = store.get_commit(&index_entry.commit_id())?;
             template.format(&commit, formatter)?;
-            // TODO: should --summary (without --patch) show diff summary as in hg log
-            // --stat?
-            if args.patch {
+            if let Some(diff_format) = diff_format {
                 show_patch(formatter, &workspace_command, &commit, diff_format)?;
             }
         }
diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs
index 12717f7f1..a4f176f17 100644
--- a/tests/test_log_command.rs
+++ b/tests/test_log_command.rs
@@ -76,4 +76,55 @@ fn test_log_with_or_without_diff() {
     +foo
 
     "###);
+
+    // `-s` implies `-p`, with or without graph
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "description", "-s"]);
+    insta::assert_snapshot!(stdout, @r###"
+    @ a new commit
+    | M file1
+    o add a file
+    | A file1
+    o 
+    "###);
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["log", "-T", "description", "--no-graph", "-s"],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    a new commit
+    M file1
+    add a file
+    A file1
+
+    "###);
+
+    // `--git` implies `-p`, with or without graph
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["log", "-T", "description", "-r", "@", "--git"],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    @ a new commit
+    ~ diff --git a/file1 b/file1
+      index 257cc5642c...3bd1f0e297 100644
+      --- a/file1
+      +++ b/file1
+      @@ -1,1 +1,2 @@
+       foo
+      +bar
+    "###);
+    let stdout = test_env.jj_cmd_success(
+        &repo_path,
+        &["log", "-T", "description", "-r", "@", "--no-graph", "--git"],
+    );
+    insta::assert_snapshot!(stdout, @r###"
+    a new commit
+    diff --git a/file1 b/file1
+    index 257cc5642c...3bd1f0e297 100644
+    --- a/file1
+    +++ b/file1
+    @@ -1,1 +1,2 @@
+     foo
+    +bar
+    "###);
 }