Re: [PATCH] diff: allow --recurse-submodules as an synonym for --submodule

From: Martin Ågren <martin.agren@gmail.com>
Date: Sep 6, 2018, 06:19 UTC
Message-ID: <CAN0heSpOG8c5En_YVvtkcwBmOrnS72cTXSw9YRJP4FG-M8dWag@mail.gmail.com>
In-Reply-To: <20180905225828.17782-1-sbeller@google.com>

On Thu, 6 Sep 2018 at 00:59, Stefan Beller <sbeller@google.com> wrote:

>
>  --submodule[=<format>]::

Maybe drop `--submodule` here ...

> +--recurse-submodules[=<format>]::
>         Specify how differences in submodules are shown.  When specifying
>         `--submodule=short` the 'short' format is used.  This format just

... and use `--recurse-submodules` here ...

>         shows the names of the commits at the beginning and end of the range.

... and mention `--submodule` here as a historical alias? Maybe deprecate it? I suppose the implementation of the aliasing is easy enough that we can carry `--submodule` around forever, though.

Show 10 quoted lines

> diff --git a/diff.c b/diff.c
> index 145cfbae592..d3d5a989bd1 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -5023,6 +5023,8 @@ int diff_opt_parse(struct diff_options *options,
>                 handle_ignore_submodules_arg(options, arg);
>         } else if (skip_to_optional_arg_default(arg, "--submodule", &arg, "log"))
>                 return parse_submodule_opt(options, arg);
> +       else if (skip_to_optional_arg_default(arg, "--recurse-submodules", &arg, "log"))
> +               return parse_submodule_opt(options, arg);

How about (whitespace-damaged)

} else if (skip_to_optional_arg_default(arg, "--submodule", &arg, "log") ||
           skip_to_optional_arg_default(arg, "--recurse-submodules",
&arg, "log"))
        return parse_submodule_opt(options, arg);

to make this future-proof? Sure, they're close enough that one should notice the two instances, and any future work work would supposedly happen in `parse_submodule_opt()` or anywhere else but here, but still.

Just a few thoughts.

Martin