All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Stephen Boyd <bebarino@gmail.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Pierre Habouzit" <madcoder@debian.org>
Subject: Re: [PATCH 06/10] parse-options: never suppress arghelp if LITERAL_ARGHELP is set
Date: Fri, 3 Dec 2010 03:40:20 -0600	[thread overview]
Message-ID: <20101203094020.GA11910@burratino> (raw)
In-Reply-To: <4CF8B55F.8020203@gmail.com>

Stephen Boyd wrote:
> On 12/01/10 15:31, Jonathan Nieder wrote:

>> The motivation is to allow update-index to correctly advertise
>> 
>> 	--cacheinfo <mode> <object> <path>
>> 	                      add the specified entry to the index
>> 
>> while abusing PARSE_OPT_NOARG to disallow the "sticked form"
>> 
>> 	--cacheinfo=<mode> <object> <path>
[...]
> parse-options should accept both forms of --cacheinfo above if the
> option isn't marked PARSE_OPT_NOARG. Marking it NOARG to get rid of the
> equals sign in the usage seems wrong when both the equals sign and no
> equals sign can be accepted.

Just to clarify: the NOARG was not meant to affect the usage message
but the actual accepted usage.  The idea was that

	git update-index --cacheinfo=100644 87a8767c87b file.c

should be rejected, because if it is accepted that would tempt people
to try

	git update-index --cacheinfo=100644 -q 87a8767c87b file.c

which fails.  That is, the argument to --cacheinfo is not <mode>,
since --cacheinfo takes _three_ arguments and therefore the sticked
form sends a wrong message.

> I know this is a bit more code, but it also means that we don't have
> this approach bite someone else down the line when they make assumptions
> about options marked as NOARG not taking arguments.
>
> We should probably add another check like "if argh is set and
> PARSE_OPT_NOARG is true error out" so this can't be done.

I agree with your conclusion.  Let's drop this patch, and I'll look
into adding the check and a PARSE_OPT_NOSTICKED flag.

  reply	other threads:[~2010-12-03  9:40 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-01 23:27 [PATCH v3 00/10] update-index: migrate to parse-options API Jonathan Nieder
2010-12-01 23:28 ` [PATCH 01/10] parse-options: Don't call parse_options_check() so much Jonathan Nieder
2010-12-05 18:14   ` René Scharfe
2010-12-06  7:57     ` Stephen Boyd
2010-12-01 23:29 ` [PATCH 02/10] parse-options: clearer reporting of API misuse Jonathan Nieder
2010-12-02  4:57   ` Jonathan Nieder
2010-12-02  6:01     ` [PATCH 02/10 v2] " Jonathan Nieder
2010-12-02  6:13   ` [PATCH 02/10 v2 resend] " Jonathan Nieder
2010-12-01 23:29 ` [PATCH 03/10] parse-options: move NODASH sanity checks to parse_options_check Jonathan Nieder
2010-12-02  6:05   ` [PATCH 03/10 v2] " Jonathan Nieder
2010-12-01 23:30 ` [PATCH 04/10] parse-options: sanity check PARSE_OPT_NOARG flag Jonathan Nieder
2010-12-02  6:08   ` [PATCH 04/10 v2] " Jonathan Nieder
2010-12-01 23:30 ` [PATCH 05/10] parse-options: do not infer PARSE_OPT_NOARG from option type Jonathan Nieder
2010-12-01 23:31 ` [PATCH 06/10] parse-options: never suppress arghelp if LITERAL_ARGHELP is set Jonathan Nieder
2010-12-03  9:16   ` Stephen Boyd
2010-12-03  9:40     ` Jonathan Nieder [this message]
2010-12-03 17:53       ` Stephen Boyd
2010-12-01 23:32 ` [PATCH 07/10] parse-options: allow git commands to invent new option types Jonathan Nieder
2010-12-01 23:32 ` [PATCH 08/10] parse-options: make resuming easier after PARSE_OPT_STOP_AT_NON_OPTION Jonathan Nieder
2010-12-01 23:33 ` [PATCH 09/10] setup: save prefix (original cwd relative to toplevel) in startup_info Jonathan Nieder
2010-12-01 23:34 ` [PATCH 10/10] update-index: migrate to parse-options API Jonathan Nieder

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=20101203094020.GA11910@burratino \
    --to=jrnieder@gmail.com \
    --cc=bebarino@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=madcoder@debian.org \
    --cc=pclouds@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.