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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux