All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Jacob Keller <jacob.keller@gmail.com>
Subject: Re: [PATCH] various: disallow --no-no-OPT for --no-opt options
Date: Wed, 19 Apr 2017 15:19:42 +0200	[thread overview]
Message-ID: <CACBZZX7Y2cqabLMkx35ZU88Y3+8zffP5hxa8haThMaErbfhcfA@mail.gmail.com> (raw)
In-Reply-To: <b4eef4c1-cd5a-aa71-fc79-8af89dbe365d@web.de>

On Wed, Apr 19, 2017 at 3:11 PM, René Scharfe <l.s.r@web.de> wrote:
> Am 19.04.2017 um 09:00 schrieb Ævar Arnfjörð Bjarmason:
>>
>> On Wed, Apr 19, 2017 at 12:29 AM, René Scharfe <l.s.r@web.de> wrote:
>>>
>>> Setting PARSE_OPT_NONEG takes away the ability to toggle the affected
>>> option.  E.g. git clone would reject --checkout.  Currently users can
>>> specify --no- options as defaults in aliases and override them on the
>>> command line if needed, with the patch that won't be possible anymore.
>>>
>>> PARSE_OPT_NONEG should only be used for options where a negation doesn't
>>> make sense, e.g. for the --stage option of checkout-index.
>>
>>
>> That's a bad bug, I don't know whether to be surprised or not that we
>> had no tests for this :)
>>
>> I thought I was just disabling --no-no-checkout for --no-checkout, not
>> --checkout, but didn't notice the subtleties of the special case
>> handling for --no-* in parse-options.c, thanks.
>
>
> I'm confused.  What's the bug here?
>
> --no-no-checkout is undocumented; Jacob's patch addresses it. --no-checkout
> is the documented form.  Negation allows --checkout to be used as well, with
> the opposite meaning to --no-checkout.  Turning off negation with
> PARSE_OPT_NONEG forbids --checkout to be used.
>
> Perhaps the issue is that a single line of documentation is not enough
> ("PARSE_OPT_NONEG: says that this option cannot be negated")?

I mean a bug in my patch, i.e. I meant to remove --no-no-OPT in cases
of --no-OPT but also removed --OPT unintentionally, but anyway, let's
drop this one, Jacob's patch is better.

  reply	other threads:[~2017-04-19 13:20 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
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 [this message]
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=CACBZZX7Y2cqabLMkx35ZU88Y3+8zffP5hxa8haThMaErbfhcfA@mail.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jacob.keller@gmail.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.