On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@xxxxxxxxx> 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. > Signed-off-by: Sergey Organov <sorganov@xxxxxxxxx> > --- > 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 >