Thread (138 messages) 138 messages, 4 authors, 2020-12-21

Re: [PATCH v2 24/33] diff-merges: handle imply -p on -c/--cc logic for log.c

From: Elijah Newren <hidden>
Date: 2020-12-18 06:02:03

On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov [off-list ref] wrote:
Move logic that handles implying -p on -c/--cc from
log_setup_revisions_tweak() to diff_merges_setup_revs(), where it
belongs.
A very minor point, but I'd probably drop the "where it belongs";
while I think the new place makes sense for it, it reads to me like
you're either relying on a consensus to move it or implying there was
a mistake to not put it here previously, neither of which makes sense.

Much more importantly, this patch doesn't do what you said in
discussions on the previous round.  It'd be helpful if the commit
message called out that you are just moving the logic for now and that
a subsequent patch will tweak the logic to only trigger this for
-c/--cc and not for --diff-merges=.* flags.

quoted hunk ↗ jump to hunk
Signed-off-by: Sergey Organov <redacted>
---
 builtin/log.c | 4 ----
 diff-merges.c | 7 ++++++-
 2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 63875c3aeec9..c3caf0955b2b 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -723,10 +723,6 @@ static void log_setup_revisions_tweak(struct rev_info *rev,
            rev->prune_data.nr == 1)
                rev->diffopt.flags.follow_renames = 1;

-       /* Turn --cc/-c into -p --cc/-c when -p was not given */
-       if (!rev->diffopt.output_format && rev->combine_merges)
-               rev->diffopt.output_format = DIFF_FORMAT_PATCH;
-
        if (rev->first_parent_only)
                diff_merges_default_to_first_parent(rev);
 }
diff --git a/diff-merges.c b/diff-merges.c
index 0165fa22fcd1..2ac25488d53e 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -127,6 +127,11 @@ void diff_merges_setup_revs(struct rev_info *revs)
                revs->first_parent_merges = 0;
        if (revs->combined_all_paths && !revs->combine_merges)
                die("--combined-all-paths makes no sense without -c or --cc");
-       if (revs->combine_merges)
+       if (revs->combine_merges) {
                revs->diff = 1;
+               /* Turn --cc/-c into -p --cc/-c when -p was not given */
+               if (!revs->diffopt.output_format)
+                       revs->diffopt.output_format = DIFF_FORMAT_PATCH;
+       }
+
 }
--
2.25.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help