| Message ID | 20210913185941.6247-1-alban.gruin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] git-clone.txt: add the --recursive option | expand |
On Mon, Sep 13, 2021 at 3:14 PM Alban Gruin <alban.gruin@gmail.com> wrote: > This adds the --recursive option, an alias of --recurse-submodule, to > git-clone's manual page. > > Signed-off-by: Alban Gruin <alban.gruin@gmail.com> > --- > I found this out when a friend told me he could not remember how to > fetch submodules with git-clone, and when another one suggested > `--recurse-submodule'. I checked the man page, and I was surprised to > find out that `--recursive' is not mentionned at all. > > I did not modify the synopsis. So, this alias, although shorter than > the "real" option, would still be somewhat hidden in the man page. Considering that the `--recursive` option was intentionally removed from `git-clone.txt` by bb62e0a99f (clone: teach --recurse-submodules to optionally take a pathspec, 2017-03-17), it's not clear that this change helps the situation.
Hi Eric, Le 13/09/2021 à 21:26, Eric Sunshine a écrit : > On Mon, Sep 13, 2021 at 3:14 PM Alban Gruin <alban.gruin@gmail.com> wrote: >> This adds the --recursive option, an alias of --recurse-submodule, to >> git-clone's manual page. >> >> Signed-off-by: Alban Gruin <alban.gruin@gmail.com> >> --- >> I found this out when a friend told me he could not remember how to >> fetch submodules with git-clone, and when another one suggested >> `--recurse-submodule'. I checked the man page, and I was surprised to >> find out that `--recursive' is not mentionned at all. >> >> I did not modify the synopsis. So, this alias, although shorter than >> the "real" option, would still be somewhat hidden in the man page. > > Considering that the `--recursive` option was intentionally removed > from `git-clone.txt` by bb62e0a99f (clone: teach --recurse-submodules > to optionally take a pathspec, 2017-03-17), it's not clear that this > change helps the situation. > The patch you mention also hides --recursive from the option array, but that was reverted with 5c387428f1 (parse-options: don't emit "ambiguous option" for aliases, 2019-04-29). The option should be re-hidden, or even removed. Alban
Eric Sunshine <sunshine@sunshineco.com> writes: > On Mon, Sep 13, 2021 at 3:14 PM Alban Gruin <alban.gruin@gmail.com> wrote: >> This adds the --recursive option, an alias of --recurse-submodule, to >> git-clone's manual page. >> >> Signed-off-by: Alban Gruin <alban.gruin@gmail.com> >> --- >> I found this out when a friend told me he could not remember how to >> fetch submodules with git-clone, and when another one suggested >> `--recurse-submodule'. I checked the man page, and I was surprised to >> find out that `--recursive' is not mentionned at all. >> >> I did not modify the synopsis. So, this alias, although shorter than >> the "real" option, would still be somewhat hidden in the man page. > > Considering that the `--recursive` option was intentionally removed > from `git-clone.txt` by bb62e0a99f (clone: teach --recurse-submodules > to optionally take a pathspec, 2017-03-17), it's not clear that this > change helps the situation. A logical continuation of what bb62e0a99f tried to do might be to hide the --recursive[=<pathspec>] from "git clone -h", I guess.
On Mon, Sep 13, 2021 at 4:42 PM Alban Gruin <alban.gruin@gmail.com> wrote: > Le 13/09/2021 à 21:26, Eric Sunshine a écrit : > > On Mon, Sep 13, 2021 at 3:14 PM Alban Gruin <alban.gruin@gmail.com> wrote: > >> This adds the --recursive option, an alias of --recurse-submodule, to > >> git-clone's manual page. > > > > Considering that the `--recursive` option was intentionally removed > > from `git-clone.txt` by bb62e0a99f (clone: teach --recurse-submodules > > to optionally take a pathspec, 2017-03-17), it's not clear that this > > change helps the situation. > > The patch you mention also hides --recursive from the option array, but > that was reverted with 5c387428f1 (parse-options: don't emit "ambiguous > option" for aliases, 2019-04-29). The option should be re-hidden, or > even removed. I don't quite follow. As far as I understand both by reading 5c387428f1 and by testing, 5c387428f1 fixed tab-completion so it would _not_ show `--recursive`. Anyhow, another approach which we've used elsewhere is to mention the option in the documentation but indicate clearly that it's deprecated. That way, people who run across the option in existing scripts or old blogs can at least find out what it means. Something like: --recurse-submodules[=<pathspec>]:: After the clone is created, initialize and clone submodules within based on the provided pathspec. If no pathspec is provided, all submodules are initialized and cloned. (`--recursive` is a deprecated synonym.) I don't have an opinion as to whether or not we'd want to do that in this case.
Hi Eric, Le 13/09/2021 à 23:57, Eric Sunshine a écrit : > On Mon, Sep 13, 2021 at 4:42 PM Alban Gruin <alban.gruin@gmail.com> wrote: >> Le 13/09/2021 à 21:26, Eric Sunshine a écrit : >>> On Mon, Sep 13, 2021 at 3:14 PM Alban Gruin <alban.gruin@gmail.com> wrote: >>>> This adds the --recursive option, an alias of --recurse-submodule, to >>>> git-clone's manual page. >>> >>> Considering that the `--recursive` option was intentionally removed >>> from `git-clone.txt` by bb62e0a99f (clone: teach --recurse-submodules >>> to optionally take a pathspec, 2017-03-17), it's not clear that this >>> change helps the situation. >> >> The patch you mention also hides --recursive from the option array, but >> that was reverted with 5c387428f1 (parse-options: don't emit "ambiguous >> option" for aliases, 2019-04-29). The option should be re-hidden, or >> even removed. > > I don't quite follow. As far as I understand both by reading > 5c387428f1 and by testing, 5c387428f1 fixed tab-completion so it would > _not_ show `--recursive`. > bb62e0a99f hid --recursive from `git clone -h' with PARSE_OPT_HIDDEN, but 5c387428f1 reverted that: $ git checkout 5c387428f1~ $ make $ bin-wrappers/git clone -h ... -s, --shared setup as shared repository --recurse-submodules[=<pathspec>] initialize submodules in the clone -j, --jobs <n> number of submodules cloned in parallel ... $ git checkout 5c387428f1 $ make $ bin-wrappers/git clone -h ... --recursive[=<pathspec>] initialize submodules in the clone --recurse-submodules[=<pathspec>] initialize submodules in the clone ... The two options were then reordered by c28b036fe3 (clone: reorder --recursive/--recurse-submodules, 2020-03-16), and this is where we are today: $ git clone -h ... --recurse-submodules[=<pathspec>] initialize submodules in the clone --recursive[=<pathspec>] alias of --recurse-submodules ... Junio did mention[0] that --recursive was no longer in the manual, but not that it was once hidden from the option list. > Anyhow, another approach which we've used elsewhere is to mention the > option in the documentation but indicate clearly that it's deprecated. > That way, people who run across the option in existing scripts or old > blogs can at least find out what it means. Something like: > > --recurse-submodules[=<pathspec>]:: > After the clone is created, initialize and clone submodules > within based on the provided pathspec. If no pathspec is > provided, all submodules are initialized and cloned. > (`--recursive` is a deprecated synonym.) > > I don't have an opinion as to whether or not we'd want to do that in this case. > [0] https://lore.kernel.org/git/20200316212857.259093-3-gitster@pobox.com/ Alban
Alban Gruin <alban.gruin@gmail.com> writes: > Junio did mention[0] that --recursive was no longer in the manual, but > not that it was once hidden from the option list. Please allow me to summarize the discussion so far. We want our subcommands to take "--recurse-submodules=<arg>" uniformly, while accepting any unique prefix, e.g. --recurs=<arg>, as its short-hand. For "git clone", we kept "--recurse=<it>" in the options[] table as a HIDDEN entry as part of our deprecation plan. This nicely hid the deprecated "--recurse=<it>" from "git clone -h". But it backfired because "git cmd --recur=<it>" was not a "unique prefix" (as it matched both) and triggered a disambiguation error. To solve it, we introduced OPT_ALIAS() to tell the machinery that allows unique prefix that these two are the same thing. As a side effect, because the use of OPT_ALIAS() did not have HIDDEN bit, we started showing the deprecated "--recurse" in "git clone -h" output. Is that where we are? I am wondering if it is just a matter of either * removing the "recursive" alias from the options table. Because we accept unique prefix, --recurse=<arg> the user types will be taken as --recurse-submodules=<arg> anyway (until "git clone" learns another option --recurse-xyzzy=<arg>, at which time it will become ambiguous and error out, that is). or * adding the PARSE_OPT_HIDDEN bit to the OPT_ALIAS() element for the deprecated "recurse" option. and all would be fine? Between adding "--recursive" to the manual and describing it as a deprecated synonym for "--recurse-submodules", and not doing so, I do not have a strong preference. Thanks.
On Tue, Sep 14, 2021 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote: > I am wondering if it is just a matter of either > > * removing the "recursive" alias from the options table. Because > we accept unique prefix, --recurse=<arg> the user types will be > taken as --recurse-submodules=<arg> anyway (until "git clone" > learns another option --recurse-xyzzy=<arg>, at which time it > will become ambiguous and error out, that is). With this option, we risk breaking existing tooling which happens to use the deprecated --recursive. > or > > * adding the PARSE_OPT_HIDDEN bit to the OPT_ALIAS() element for > the deprecated "recurse" option. I was going to suggest this as a possible way forward to address Alban's most recent response to my response. The lack of PARSE_OPT_HIDDEN on OPT_ALIAS() almost seems like an oversight. > Between adding "--recursive" to the manual and describing it as a > deprecated synonym for "--recurse-submodules", and not doing so, I > do not have a strong preference. I don't have a strong preference either, especially considering how long ago --recursive was removed from the manual, however, adding it would help someone who runs across --recursive in existing tooling or old blog post and wants to know what it does.
Eric Sunshine <sunshine@sunshineco.com> writes: > On Tue, Sep 14, 2021 at 1:46 PM Junio C Hamano <gitster@pobox.com> wrote: >> I am wondering if it is just a matter of either >> >> * removing the "recursive" alias from the options table. Because >> we accept unique prefix, --recurse=<arg> the user types will be >> taken as --recurse-submodules=<arg> anyway (until "git clone" >> learns another option --recurse-xyzzy=<arg>, at which time it >> will become ambiguous and error out, that is). > > With this option, we risk breaking existing tooling which happens to > use the deprecated --recursive. Ahh, sorry and thanks for correcting my stupid thinko. recursive is not a prefix of recurse-submodules. >> * adding the PARSE_OPT_HIDDEN bit to the OPT_ALIAS() element for >> the deprecated "recurse" option. > > I was going to suggest this as a possible way forward to address > Alban's most recent response to my response. The lack of > PARSE_OPT_HIDDEN on OPT_ALIAS() almost seems like an oversight. You may have an alias with no intention to deprecate either, so it would make it cumbersome if OPT_ALIAS() always meant HIDDEN, just like it currently is cumbersome for an alias that is deprecated. Independently (because I do not think this helps in solving the current situation), we might want to tweak the disambiguation machinery to require HIDDEN ones to be spelled out exactly, because they are hidden for a reason---we do not want users to casually and accidentally trigger them. Of course, that is totally outside the scope of everything we discussed so far. >> Between adding "--recursive" to the manual and describing it as a >> deprecated synonym for "--recurse-submodules", and not doing so, I >> do not have a strong preference. > > I don't have a strong preference either, especially considering how > long ago --recursive was removed from the manual, however, adding it > would help someone who runs across --recursive in existing tooling or > old blog post and wants to know what it does. Makes sense. I think we list other deprecated ones for that exact reason.
diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index 3fe3810f1c..8a578252a0 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -270,6 +270,7 @@ branch. This is useful e.g. to maintain minimal clones of the default branch of some repository for search indexing. --recurse-submodules[=<pathspec>]:: +--recursive[=<pathspec>]:: After the clone is created, initialize and clone submodules within based on the provided pathspec. If no pathspec is provided, all submodules are initialized and cloned.
This adds the --recursive option, an alias of --recurse-submodule, to git-clone's manual page. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> --- I found this out when a friend told me he could not remember how to fetch submodules with git-clone, and when another one suggested `--recurse-submodule'. I checked the man page, and I was surprised to find out that `--recursive' is not mentionned at all. I did not modify the synopsis. So, this alias, although shorter than the "real" option, would still be somewhat hidden in the man page. Documentation/git-clone.txt | 1 + 1 file changed, 1 insertion(+)