On Fri, Sep 10 2021, Eric Sunshine wrote: > On Wed, Sep 8, 2021 at 3:40 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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@xxxxxxxxx/ > > 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. > [1]: https://lore.kernel.org/git/f8560b11-ba56-0a52-8bb4-5b71f0657764@xxxxxxxxxxxxxx/ > >> 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.
