All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Git Mailing List" <git@vger.kernel.org>
Subject: Re: [PATCH] various: disallow --no-no-OPT for --no-opt options
Date: Wed, 19 Apr 2017 11:05:56 -0400	[thread overview]
Message-ID: <20170419150556.ycydwyoi27j7veva@sigill.intra.peff.net> (raw)
In-Reply-To: <CACBZZX5wHvWgqjBaiD4pJsVQug=+-4xRwkvfZ0uZ=meE3xR6NQ@mail.gmail.com>

On Wed, Apr 19, 2017 at 09:02:52AM +0200, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Apr 19, 2017 at 4:50 AM, Jeff King <peff@peff.net> wrote:
> > On Tue, Apr 18, 2017 at 07:40:37PM -0700, Junio C Hamano wrote:
> >
> >> > It might even be possible to detect the existing line and
> >> > have parse-options automatically respect "--foo" when "--no-foo" is
> >> > present.  But that may run afoul of callers that add both "--foo" and
> >> > "--no-foo" manually.
> >>
> >> True but wouldn't that something we would want to avoid anyway?
> >> That is, "git cmd [--OPT | --no-OPT | --no-no-OPT]" from the end
> >> user's point of view should be an error because it is unclear what
> >> difference there are between --OPT and --no-no-OPT.  And we should
> >> be able to add a rule to parse_options_check() to catch such an
> >> error.
> >
> > I meant that if you have something like this in your options array:
> >
> >   { 0, "foo", OPTION_INTEGER, &foo, 1 },
> >   { 0, "no-foo", OPTION_INTEGER, &foo, 2 },
> 
> I may be missing something, but don't we already do exactly what
> you're describing here? See commit f1930c587 ("parse-options: allow
> positivation of options starting, with no-", 2012-02-25) from René
> Scharfe.

Oh, so we do. I somehow totally missed that.

I think all is well, then. We do accept --no-no-foo still. IMHO it is
not that big a deal (as long as we do not advertise it, then it does not
hurt unless somebody actually tries it). But I do not mind if it goes
away, as long as the positive --foo form still works (and it sounds from
Jake's response that we'd need to do more than just NONEG there).

-Peff

  parent reply	other threads:[~2017-04-19 15:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-18 17:09 [PATCH] various: disallow --no-no-OPT for --no-opt options Ævar Arnfjörð Bjarmason
2017-04-18 22:29 ` René Scharfe
2017-04-19  1:41   ` Jeff King
2017-04-19  2:40     ` Junio C Hamano
2017-04-19  2:50       ` Jeff King
2017-04-19  7:02         ` Ævar Arnfjörð Bjarmason
2017-04-19  9:08           ` Jacob Keller
2017-04-19 15:05           ` Jeff King [this message]
2017-04-19  7:00   ` Ævar Arnfjörð Bjarmason
2017-04-19 13:11     ` René Scharfe
2017-04-19 13:19       ` Ævar Arnfjörð Bjarmason
2017-04-19 13:44         ` René Scharfe

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=20170419150556.ycydwyoi27j7veva@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    /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.