Re: [PATCH v2 22/33] diff-merges: implement new values for --diff-merges

From: Elijah Newren <newren@gmail.com>
Date: Dec 18, 2020, 05:39 UTC
Message-ID: <CABPp-BG1S83EUbqtqqjeLT4=Je44BpRMrf0MB+DNQK2wjRDbqQ@mail.gmail.com>
In-Reply-To: <20201216184929.3924-23-sorganov@gmail.com>

On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@gmail.com> wrote:

Show 40 quoted lines

>
> We first implement new options as exact synonyms for their original
> counterparts, to get all the infrastructure right, and keep functional
> improvements for later commits.
>
> The following values are implemented:
>
> --diff-merges=          old equivalent
> first|first-parent    = --first-parent (only format implications)
> sep|separate          = -m
> comb|combined         = -c
> dense| dense-combined = --cc
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  diff-merges.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/diff-merges.c b/diff-merges.c
> index 6446e2093661..cba391604ac7 100644
> --- a/diff-merges.c
> +++ b/diff-merges.c
> @@ -15,6 +15,11 @@ static void set_separate(struct rev_info *revs) {
>         revs->separate_merges = 1;
>  }
>
> +static void set_first_parent(struct rev_info *revs) {
> +       set_separate(revs);
> +       revs->first_parent_merges = 1;
> +}
> +
>  static void set_m(struct rev_info *revs) {
>         /*
>          * To "diff-index", "-m" means "match missing", and to the "log"
> @@ -38,11 +43,19 @@ static void set_dense_combined(struct rev_info *revs) {
>  }
>
>  static void set_diff_merges(struct rev_info *revs, const char *optarg) {
> -       if (!strcmp(optarg, "off")) {
> +       if (0) ;

Leftover cruft from some intermediate changes or something?

Show 12 quoted lines

> +       else if (!strcmp(optarg, "off")   || !strcmp(optarg, "none"))
>                 suppress(revs);
> -       } else {
> +       else if (!strcmp(optarg, "first") || !strcmp(optarg, "first-parent"))
> +               set_first_parent(revs);
> +       else if (!strcmp(optarg, "sep")   || !strcmp(optarg, "separate"))
> +               set_separate(revs);
> +       else if (!strcmp(optarg, "comb")  || !strcmp(optarg, "combined"))
> +               set_combined(revs);
> +       else if (!strcmp(optarg, "dense") || !strcmp(optarg, "dense-combined"))
> +               set_dense_combined(revs);
> +       else

Not sure I like the special-casing for "sep" and "comb". "dense" seems okay since it's a real word. Since you're adding short versions of m, c, and cc later in the series, do we need these other special-case forms?

Show 8 quoted lines

>                 die(_("unknown value for --diff-merges: %s"), optarg);
> -       }
>  }
>
>  /*
> --
> 2.25.1
>