On Wed, Aug 23, 2023 at 1:37 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
>
> On Tue, Aug 22, 2023 at 09:28:37PM -0600, Alex Henrie wrote:
> > + if (!*arg) {
> > + warning(_("--recurse-submodules with an empty string "
> > + "argument is deprecated and will stop "
> > + "working in a future version of Git. Use "
> > + "--recurse-submodules without an argument "
> > + "instead, which does the same thing."));
>
> This advice says to use `--recurse-submodules` as a non-deprecated
> synonym for `--recurse-submodules=""`, but I am not so sure that is
> correct advice.
>
> In the pre-image of this patch, having arg be set to the empty string
> would cause us to fall into the path that executes
>
> *v = parse_fetch_recurse_submodules_arg(opt->long_name, arg);
>
> which calls `parse_fetch_recurse()` -> `git_parse_maybe_bool()` ->
> `git_parse_maybe_bool_text()` which given the empty string will return
> 0.
>
> So here we'd be doing the equivalent of
>
> *v = RECURSE_SUBMODULES_OFF;
>
> when trying to parse `--recurse-submodules=""`. Should this advice
> instead say "[...] Use --no-recurse-submodules without an argument,
> which does the same thing"?
You're right; I misunderstood the situation here.
--recurse-submodules="" is indeed equivalent to
--no-recurse-submodules, and that's what the advice should recommend.
On the other hand, given that the empty string does the same thing
both in a config file and on the command line, maybe it's not a
problem to allow the empty string on the command line. Personally I
think I'd still prefer to ask the user to use a more explicit syntax.
Thoughts?
-Alex
