Re: parse-options: ambiguous LASTARG_DEFAULT and OPTARG

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Jun 6, 2009, 10:30 UTC
Message-ID: <4A2A4534.80604@lsrfire.ath.cx>
In-Reply-To: <4A28B072.8030006@gmail.com>

Stephen Boyd schrieb:

Show 20 quoted lines

> 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?

Show 16 quoted lines

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