Re: [PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array

3 messages, 2 authors, 2021-09-11 · open the first message on its own page

Re: [PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array

From: Junio C Hamano <hidden>
Date: 2021-09-11 00:21:27

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
The strings in the the "usagestr" array have special handling for the
empty string dating back to f389c808b67 (Rework make_usage to print
the usage message immediately, 2007-10-14).

We'll prefix all strings after the first one with "or: ". Then if we
encountered a "" we'll emit all strings after that point verbatim
without any "or: " prefixing.

This gets rid of that special case, which was added in
f389c808b67 (Rework make_usage to print the usage message immediately,
2007-10-14). It was only used "blame" (the "credential-cache*" use of
used *by* "blame"
it was removed in the preceding commit). Before this change we'd emit:

    $ git blame -h
    usage: git blame [<options>] [<rev-opts>] [<rev>] [--] <file>

        <rev-opts> are documented in git-rev-list(1)
The most crucial information is missing.  It may be shared with the
previous step but it is even worse in this step.

Why is this loss of feature a desirable thing in the first place?

What are we gaining by breaking the "after listing the alternative
forms concatenated with 'or:', we can give a general description,
before going onto the list of options"?

Without that explained, the remainder of the proposed log message
reads more like an excuse for breaking the feature, justifying that
the loss of feature can be worked around, than the description of a
solution.

Thanks.

Re: [PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-11 02:49:28

On Fri, Sep 10 2021, Junio C Hamano wrote:
Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted
The strings in the the "usagestr" array have special handling for the
empty string dating back to f389c808b67 (Rework make_usage to print
the usage message immediately, 2007-10-14).

We'll prefix all strings after the first one with "or: ". Then if we
encountered a "" we'll emit all strings after that point verbatim
without any "or: " prefixing.

This gets rid of that special case, which was added in
f389c808b67 (Rework make_usage to print the usage message immediately,
2007-10-14). It was only used "blame" (the "credential-cache*" use of
used *by* "blame"
quoted
it was removed in the preceding commit). Before this change we'd emit:

    $ git blame -h
    usage: git blame [<options>] [<rev-opts>] [<rev>] [--] <file>

        <rev-opts> are documented in git-rev-list(1)
The most crucial information is missing.  It may be shared with the
previous step but it is even worse in this step.

Why is this loss of feature a desirable thing in the first place?

What are we gaining by breaking the "after listing the alternative
forms concatenated with 'or:', we can give a general description,
before going onto the list of options"?

Without that explained, the remainder of the proposed log message
reads more like an excuse for breaking the feature, justifying that
the loss of feature can be worked around, than the description of a
solution.
Making it the same as how "git bundle" describes it doesn't seem
anywhere close to breaking it, and that also goes to the point you had
about brevity on another patch.

I think that passing custom or generated help advice here might make
sense generally, i.e. we might eventually want to expand that to all
built-in as part of improving the -h and --help output, see e.g. the
note at the bottom of "git --help".

But if we do that let's do that with an API where we simply pass this
custom string in as another parameter to the function, rather than
having a state machine to parse it out of the array we use for the
"usage:" and "or:" list of lines.

Re: [PATCH v2 3/6] parse-options: stop supporting "" in the usagestr array

From: Junio C Hamano <hidden>
Date: 2021-09-11 06:52:53

Ævar Arnfjörð Bjarmason [off-list ref] writes:
But if we do that let's do that with an API where we simply pass this
custom string in as another parameter to the function, rather than
having a state machine to parse it out of the array we use for the
"usage:" and "or:" list of lines.
Sorry, but I do not follow.  A perfectly fine way to encode the
three (usage:, or:, and text) kind of information in a single array
is what this step is breaking, and then you propose to keep the two
still in that same array, with only the third kind kicked out to
"another parameter", which does not make much sense to me.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help