Re: [PATCH] usage: clarify --recurse-submodules as a boolean

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 7, 2023, 23:47 UTC
Message-ID: <xmqqcz4fi7bd.fsf@gitster.g>
In-Reply-To: <ZDCWrl4GhgYKYFYG@google.com>

Emily Shaffer <nasamuffin@google.com> writes:

> `git switch` `git checkout`, `git reset`, and `git read-tree` allow a user to choose to
> recurse into submodules. All three of these commands' short usage seems
> to indicate that `--recurse-submodules` should take an argument. In
> practice, ...

Did you add 'git switch' at the last minute in so much of a hurry that you forgot to put a comma after it, or rewrap the paragraph? ;-)

I do agree with you that "git checkout -h" and "git reset -h" that list

	--recurse-submodules[=<checkout>]
	--recurse-submodules[=<reset>]

are being unnecessarily confusing by not saying anything about what these placeholders are to be filled with.

This however is a breaking change. Even though there is no hint that <checkout> and <reset> placeholders above take either Boolean true or false in the documentation, they may have picked up a habit to use the undocumented form from some random website. I am not sure it is safe to change the behaviour right under them, like this patch does, and I wonder if we should do this in two steps, with its first step doing:

 * "--[no-]recurse-submodules" from the command line gets no
   warning, as that is the way we recommend users to use the
   feature.

 * "--recurse-submodules=$true" and "--recurse-submodules=$false"
   (for various ways to spell true and false) get warning that tells
   the users that versions of Git in a year or more in the future
   will stop supporting the Boolean argument form of the option and
   instructs them to use "--[no-]recurse-submodules" instead.

We may have to also mention in the documentation that historically the code accepted a Boolean value as an optional argument for the option by mistake, but we are deprecating that form.

And after the second step, the code will end up looking like what this patch shows.

Thanks.