Re: [RFH] convert shortlog to use parse_options

[Junio C Hamano <gitster@pobox.com>]

Pierre Habouzit <madcoder@debian.org> writes:

> I thought of that, but it's really convoluted and can definitely lead
> to very subtle issues. The number of git commands with optional
> arguments is quite low, mostly due to legacy, I don't expect _new_
> commands to take optional arguments.

I do not think we want to assume anything like that for a generic API.

> I don't really like the ambiguity
> it creates, and in some cases you just won't be able to disambiguate at
> all. Here it looks nice because --abbrev takes an integer argument, and
> it's likely that no branch nor reference names will be only made of
> digits. Though for commands taking an optional string[0] argument this is
> way more fishy.

I thought about ambiguity issues and I was only 70% sure about my
suggestion.  If you have an object, the initial unique part of whose
name consists only of decimal digits, you could get:

	git describe --abbrev 7 538538538
	git describe --abbrev 538538538 HEAD
	git describe --abbrev 538538538

and the last case needs to be disambiguated (either show HEAD with
abbreviation of 540 million digits, or show that object abbreviated to
the default length).  Because we are discussing a generic "optional
integer with default value" parser, let's not argue that only a small
integer between 0..40 makes sense to --abbrev flag in this context and
we should use that knowledge to further disambiguate [*1*].

You could say "If a flag takes an optional parameter and has a default,
an empty string means use the default", and disambiguate the lat example
in the above this way:

	git describe --abbrev 538538538
	git describe --abbrev '' 538538538

But it is not prettier than always requiring the optional parameter to
be sticked with the flag, as you suggest, like this;

	git describe --abbrev 538538538
	git describe --abbrev=538538538

So I am not entirely opposed to your version, nor I am claiming my
suggestion is better.  However, I just thought that "some parameters you
MUST stick to the flag" might create confusion to the end users, and I
wanted to see if others can come up with a less confusing alternative.
And the way I did so was to keep the discussion going by stirring the
pot a bit.

>   [0] OTOH I'm not sure there will ever be optional arguments that
>       aren't integers in git, but I may be wrong.

We could make HEAD as the default for "git branch --contains", if you
want an example.

[Footnote]

*1* We could introduce "optional integer with valid range with default",
and that would fit naturally into the scheme I outlined.  Even if the
next token parses as an integer, if it is out of range, you can say it
is not yours, and if it has to be yours, you can barf, saying "that's
out of range".

"The valid range" would be useful regardless of disambiguation. I wished
for "only valid is 1 or more" when I adjusted one of the commands to
parse_options().