Re: parse-options: ambiguous LASTARG_DEFAULT and OPTARG

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.