All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Git List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>
Subject: Re: [PATCH 5/6] help: correct logic error in combining --all and --config
Date: Fri, 10 Sep 2021 04:08:50 -0400	[thread overview]
Message-ID: <CAPig+cROO1VT9Umdtor9zc8eftyDTBg+ER2igZG+jxygM9gLJw@mail.gmail.com> (raw)
In-Reply-To: <87bl52hn40.fsf@evledraar.gmail.com>

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).

[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.

  reply	other threads:[~2021-09-10  8:09 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-08 15:24 [PATCH 0/6] help: fix usage nits & bugs, completion shellscript->C Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 1/6] help: correct the usage string in -h and documentation Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 2/6] help: correct usage string for "git help --guides" Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 3/6] help tests: add test for --config output Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 4/6] help: refactor "for_human" control flow in cmd_help() Ævar Arnfjörð Bjarmason
2021-09-08 16:41   ` Eric Sunshine
2021-09-08 17:02     ` Junio C Hamano
2021-09-08 19:36       ` Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 5/6] help: correct logic error in combining --all and --config Ævar Arnfjörð Bjarmason
2021-09-08 16:39   ` Eric Sunshine
2021-09-08 19:37     ` Ævar Arnfjörð Bjarmason
2021-09-10  8:08       ` Eric Sunshine [this message]
2021-09-10 11:09         ` Ævar Arnfjörð Bjarmason
2021-09-08 15:24 ` [PATCH 6/6] help / completion: make "git help" do the hard work Ævar Arnfjörð Bjarmason
2021-09-10 11:28 ` [PATCH v2 0/5] help: fix usage nits & bugs, completion shellscript->C Ævar Arnfjörð Bjarmason
2021-09-10 11:28   ` [PATCH v2 1/5] help: correct the usage string in -h and documentation Ævar Arnfjörð Bjarmason
2021-09-11  1:12     ` Junio C Hamano
2021-09-11  2:34       ` Ævar Arnfjörð Bjarmason
2021-09-10 11:28   ` [PATCH v2 2/5] help: correct usage & behavior of "git help --guides" Ævar Arnfjörð Bjarmason
2021-09-10 18:15     ` Philip Oakley
2021-09-10 18:21       ` Philip Oakley
2021-09-21 13:49         ` Ævar Arnfjörð Bjarmason
2021-09-21 14:20           ` Philip Oakley
2021-09-11  1:22     ` Junio C Hamano
2021-09-10 11:28   ` [PATCH v2 3/5] help tests: add test for --config output Ævar Arnfjörð Bjarmason
2021-09-11  1:32     ` Junio C Hamano
2021-09-11  2:25       ` Ævar Arnfjörð Bjarmason
2021-09-13 19:21     ` Philip Oakley
2021-09-10 11:28   ` [PATCH v2 4/5] help: correct logic error in combining --all and --config Ævar Arnfjörð Bjarmason
2021-09-10 23:45     ` Junio C Hamano
2021-09-10 11:28   ` [PATCH v2 5/5] help / completion: make "git help" do the hard work Ævar Arnfjörð Bjarmason
2021-09-11  1:35     ` Junio C Hamano
2021-09-21 22:40   ` [PATCH v3 0/9] help: fix usage nits & bugs, completion shellscript->C Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 1/9] help: correct the usage string in -h and documentation Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 2/9] help: correct usage & behavior of "git help --guides" Ævar Arnfjörð Bjarmason
2021-09-23 18:05       ` Junio C Hamano
2021-09-21 22:40     ` [PATCH v3 3/9] help tests: add test for --config output Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 4/9] help: correct logic error in combining --all and --config Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 5/9] help: correct logic error in combining --all and --guides Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 6/9] help: simplify by moving to OPT_CMDMODE() Ævar Arnfjörð Bjarmason
2021-09-23 18:03       ` Junio C Hamano
2021-09-23 18:05       ` Junio C Hamano
2021-09-21 22:40     ` [PATCH v3 7/9] help tests: test --config-for-completion option & output Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 8/9] help / completion: make "git help" do the hard work Ævar Arnfjörð Bjarmason
2021-09-21 22:40     ` [PATCH v3 9/9] help: move column config discovery to help.c library Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPig+cROO1VT9Umdtor9zc8eftyDTBg+ER2igZG+jxygM9gLJw@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=philipoakley@iee.email \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.