git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Stephen Boyd <bebarino@gmail.com>
Cc: git list <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>,
	Pierre Habouzit <madcoder@madism.org>
Subject: Re: parse-options: ambiguous LASTARG_DEFAULT and OPTARG
Date: Sat, 06 Jun 2009 12:30:12 +0200	[thread overview]
Message-ID: <4A2A4534.80604@lsrfire.ath.cx> (raw)
In-Reply-To: <4A28B072.8030006@gmail.com>

Stephen Boyd schrieb:
> Hi,
> 
> This in builtin-branch.c
> 
>         {
> 		OPTION_CALLBACK, 0, "merged", &merge_filter_ref,
> 		"commit", "print only merged branches",
> 		PARSE_OPT_LASTARG_DEFAULT | PARSE_OPT_NONEG,
> 		opt_parse_merge_filter, (intptr_t) "HEAD",
> 	},
> 
> and the usage message for "git-branch -h" will print out
> 
>     --merged <commit>
> 
> when I'm expecting
> 
>     --merged[=<commit>]
> 
> This is because the PARSE_OPT_OPTARG flag is not used. Is this correct?

> The default value is still set correctly in some cases, but become
> ambiguous in other cases. Take this for example
> 
>     $ git branch --merged --verbose
>     fatal: malformed object name --verbose
> 
> but
> 
>     $ git branch --verbose --merged
> 
> works fine.
> 
> The simple fix is to just add PARSE_OPT_OPTARG to the flags, and fix a
> test or two. But I'm wondering if doing that will become problematic for
> end-users. Essentially you can no longer do git branch --merged master,
> you must do git branch --merged=master.

PARSE_OPT_OPTARG overrides PARSE_OPT_LASTARG_DEFAULT, as Pierre noted in
commit 1cc6985c, which introduced the latter, so the two should not be
used together.

PARSE_OPT_LASTARG_DEFAULT uses the default value if the option is the
last one on the command line and requires an explicit argument if it's
not the last, as you found out above.  That's also what the code says 
and its name implies; the comment in parse-options.h (by yours truly) is 
probably misleading because it doesn't mention this condition.

I don't remember any other program having options with such a behaviour; 
I'm not sure how to stress that --merged needs to be the last option, as 
implied by the help message.

  reply	other threads:[~2009-06-06 10:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-05  5:43 parse-options: ambiguous LASTARG_DEFAULT and OPTARG Stephen Boyd
2009-06-06 10:30 ` René Scharfe [this message]
2009-06-06 20:14   ` Stephen Boyd
2009-06-07 23:39     ` [PATCH] show-branch: don't use LASTARG_DEFAULT with OPTARG Stephen Boyd
2009-06-08 17:24       ` René Scharfe
2009-06-08 21:56       ` Junio C Hamano
2009-06-09  8:23         ` [PATCH] parse-options: add parse_options_check to validate option specs Pierre Habouzit
2009-06-12 19:31           ` Pierre Habouzit
2009-06-12 21:25             ` 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=4A2A4534.80604@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=bebarino@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@madism.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).