Re: [PATCH v2 20/35] doc txt & -h consistency: add missing options and labels
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Sep 29, 2022, 03:04 UTC
- Message-ID
- <CAPig+cS01QEcAvr0yqhdhGc8MtvHMofKOHSeJZy2yFqq0Oqo8Q@mail.gmail.com>
- In-Reply-To
- <patch-v2-20.35-1c2bc223c5d-20220928T082458Z-avarab@gmail.com>
On Wed, Sep 28, 2022 at 4:43 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 15 quoted lines
> Fix various issues of SYNOPSIS and -h output syntax where: > > * Options such as --force were missing entirely > * ...or the short option, such as -f > > * We said "opts" or "options", but could instead enumerate > the (small) set of supported options > > * argument labels were missing entirely (ls-remote) > > * How we referred to an argument was inconsistent between the two, > e.g. <pack> v.s. <pack>.idx. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > ---
A few minor comments (probably not worth a re-roll)...
Show 5 quoted lines
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -88,7 +88,7 @@ static struct option builtin_help_options[] = {
> static const char * const builtin_help_usage[] = {
> - "git help [-a|--all] [--[no-]verbose]] [--[no-]external-commands] [--[no-]aliases]",
> + "git help [-a|--all] [--[no-]verbose] [--[no-]external-commands] [--[no-]aliases]",This is merely removing an extra "]", thus should it instead be bundled along with [12/35] (after generalizing the commit message of [12/35] a bit)?
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> @@ -20,6 +20,7 @@ static const char * const send_pack_usage[] = {
> " [--verbose] [--thin] [--atomic]\n"
> + " [--[no-]signed|--signed=(true|false|if-asked)]\n"Should there be spaces around the leftmost vertical bar per [14/35]?
[--[no-]signed | --signed=(true|false|if-asked)]
Show 5 quoted lines
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> @@ -20,7 +20,7 @@
> static char const * const builtin_sparse_checkout_usage[] = {
> - N_("git sparse-checkout (init | list | set | add | reapply | disable) <options>"),
> + N_("git sparse-checkout (init | list | set | add | reapply | disable) [<options>]"),Does this change belong in [15/35] where you also add "[" and "]" around "<options>" for the 'mv' command? Citing [15/35]:
In the case of "mv" let's add the missing "[]" to indicate that these
are optional.Show 5 quoted lines
> diff --git a/builtin/verify-pack.c b/builtin/verify-pack.c
> @@ -56,7 +56,7 @@ static int verify_one_pack(const char *path, unsigned int flags, const char *has
> static const char * const verify_pack_usage[] = {
> - N_("git verify-pack [-v | --verbose] [-s | --stat-only] <pack>..."),
> + N_("git verify-pack [-v | --verbose] [-s | --stat-only] [--] <pack>.idx..."),Part of this change makes sense in this patch, but doesn't the "[--]" part belong in [18/35]?