git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 5/6] help: correct logic error in combining --all and --config

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 10, 2021, 11:09 UTC
Message-ID
<87y284d6nc.fsf@evledraar.gmail.com>
In-Reply-To
<CAPig+cROO1VT9Umdtor9zc8eftyDTBg+ER2igZG+jxygM9gLJw@mail.gmail.com>
On Fri, Sep 10 2021, Eric Sunshine wrote:
Show 22 quoted lines
> On Wed, Sep 8, 2021 at 3:40 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>> On Wed, Sep 08 2021, Eric Sunshine wrote:
>> > I personally find it highly frustrating when a program merely dumps
>> > the usage statement without any explanation of what exactly it doesn't
>> > like about the command-line. Printing out a simple:
>> >
>> >     --all, --guides, --config are mutually exclusive
>> >
>> > message would go a long way toward reducing the frustration level.
>>
>> I'll make it emit something more helpful.
>>
>> More generally I've got quite a bit of parse_options() improvements
>> queued up locally that I've been trying to trickle in at the rate I can
>> get them through the list, review over at [1] would be much appreciated.
>> https://lore.kernel.org/git/cover-0.2-00000000000-20210901T110917Z-avarab@gmail.com/
>
> My review time is very limited these days (which is why most of my
> review comments lately are superficial), but I set aside some time to
> review that series for you[1]. Most of my review is subjective, but I
> did identify one lurking bug (assuming I understand the code
> correctly).
Thanks, I'll try to take a look at that in detail & re-roll soon.
Show 32 quoted lines
> [1]: https://lore.kernel.org/git/f8560b11-ba56-0a52-8bb4-5b71f0657764@sunshineco.com/
>
>> I wonder if we can do this automatically, we already have the
>> builtin_help_usage, we could parse that in the general case and find
>> that certain options are mutually exclusive per the examples there.
>>
>> We'd then discover what option we parsed when usage_with_options() was
>> called, and automatically emit a useful message in these sorts of cases.
>>
>> Of course the usage strings might be incomplete or wrong, but part of
>> the point would be to find those cases & a test mode to die() if a
>> command was called with some option combinations not suggested as
>> working according to its documented usage...
>
> Perhaps, though I imagine it would have to employ some, um,
> "interesting" heuristics, and be quite hit-and-miss at first, at least
> until people get around to formalizing existing and new usage strings
> with the specific goal of supporting that feature.
>
> Speaking of heuristics and wishful thinking, when I read the cover
> letter of your series which I just reviewed, I thought at first that
> the end-goal would be to ignore whatever indentation the caller
> provided following each embedded newline, and instead insert the
> correct computed indentation automatically. This approach would
> obviate the need for the [1/2] indentation cleanup patch. However,
> doing so would require heuristics or at least manual help from the
> caller to indicate the proper indentation width. I also thought
> perhaps the intention of the series was to do the line-wrapping
> automatically (ignoring any caller-provided embedded newlines), thus
> ensuring that the lines were indented correctly _and_ fit the terminal
> width properly regardless, but that's a somewhat more substantial
> change.

Yeah, I considered it but decided not to mainly not for the heuristics reason, but that any such thing means that you won't have the same indentation in the C code. I.e. instead of:

    "git foo [--some-option]\n"
    "        [-even -more options]"
You'd have:
    "git foo [--some-option]\n"
    "[--even --more options]"

Which I think is just not as legible, but also in the general case you get into the quagmire of how do align a thing like:

    "git [-c foo=bar] something [--option]\n"
    "[--more-options]"

Are we doing to align on the "git " or "git [-c foo=bar] something ", or is it a "git something" where "something" is the name of a sub-command, or is that a filename?

We control the source code so we can do it, but I thought it would be nasty, and in any case any working solution wouldn't give you alignment in the source code, so I dropped the idea.

Previous: Eric SunshineNext: Ævar Arnfjörð Bjarmason
Message 13 of 45 in “help: fix usage nits & bugs, completion shellscript->C”
  1. 0/6 help: fix usage nits & bugs, completion shellscript->CÆvar Arnfjörð Bjarmason, Sep 8, 2021
  2. 1/6 help: correct the usage string in -h and documentationÆvar Arnfjörð Bjarmason, Sep 8, 2021
  3. 2/6 help: correct usage string for "git help --guides"Ævar Arnfjörð Bjarmason, Sep 8, 2021
  4. 3/6 help tests: add test for --config outputÆvar Arnfjörð Bjarmason, Sep 8, 2021
  5. 4/6 help: refactor "for_human" control flow in cmd_help()Ævar Arnfjörð Bjarmason, Sep 8, 2021
  6. Eric SunshineSep 8, 2021
  7. Junio C HamanoSep 8, 2021
  8. Ævar Arnfjörð BjarmasonSep 8, 2021
  9. 5/6 help: correct logic error in combining --all and --configÆvar Arnfjörð Bjarmason, Sep 8, 2021
  10. Eric SunshineSep 8, 2021
  11. Ævar Arnfjörð BjarmasonSep 8, 2021
  12. Eric SunshineSep 10, 2021
  13. Ævar Arnfjörð BjarmasonSep 10, 2021
  14. 6/6 help / completion: make "git help" do the hard workÆvar Arnfjörð Bjarmason, Sep 8, 2021
  15. 0/5 help: fix usage nits & bugs, completion shellscript->CÆvar Arnfjörð Bjarmason, Sep 10, 2021
  16. 1/5 help: correct the usage string in -h and documentationÆvar Arnfjörð Bjarmason, Sep 10, 2021
  17. Junio C HamanoSep 11, 2021
  18. Ævar Arnfjörð BjarmasonSep 11, 2021
  19. 2/5 help: correct usage & behavior of "git help --guides"Ævar Arnfjörð Bjarmason, Sep 10, 2021
  20. Philip OakleySep 10, 2021
  21. Philip OakleySep 10, 2021
  22. Ævar Arnfjörð BjarmasonSep 21, 2021
  23. Philip OakleySep 21, 2021
  24. Junio C HamanoSep 11, 2021
  25. 3/5 help tests: add test for --config outputÆvar Arnfjörð Bjarmason, Sep 10, 2021
  26. Junio C HamanoSep 11, 2021
  27. Ævar Arnfjörð BjarmasonSep 11, 2021
  28. Philip OakleySep 13, 2021
  29. 4/5 help: correct logic error in combining --all and --configÆvar Arnfjörð Bjarmason, Sep 10, 2021
  30. Junio C HamanoSep 10, 2021
  31. 5/5 help / completion: make "git help" do the hard workÆvar Arnfjörð Bjarmason, Sep 10, 2021
  32. Junio C HamanoSep 11, 2021
  33. 0/9 help: fix usage nits & bugs, completion shellscript->CÆvar Arnfjörð Bjarmason, Sep 21, 2021
  34. 1/9 help: correct the usage string in -h and documentationÆvar Arnfjörð Bjarmason, Sep 21, 2021
  35. 2/9 help: correct usage & behavior of "git help --guides"Ævar Arnfjörð Bjarmason, Sep 21, 2021
  36. Junio C HamanoSep 23, 2021
  37. 3/9 help tests: add test for --config outputÆvar Arnfjörð Bjarmason, Sep 21, 2021
  38. 4/9 help: correct logic error in combining --all and --configÆvar Arnfjörð Bjarmason, Sep 21, 2021
  39. 5/9 help: correct logic error in combining --all and --guidesÆvar Arnfjörð Bjarmason, Sep 21, 2021
  40. 6/9 help: simplify by moving to OPT_CMDMODE()Ævar Arnfjörð Bjarmason, Sep 21, 2021
  41. Junio C HamanoSep 23, 2021
  42. Junio C HamanoSep 23, 2021
  43. 7/9 help tests: test --config-for-completion option & outputÆvar Arnfjörð Bjarmason, Sep 21, 2021
  44. 9/9 help: move column config discovery to help.c libraryÆvar Arnfjörð Bjarmason, Sep 21, 2021
  45. 8/9 help / completion: make "git help" do the hard workÆvar Arnfjörð Bjarmason, Sep 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.