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

["Martin Ågren" <martin.agren@gmail.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.

> 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