Re: [RFH] convert shortlog to use parse_options

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 13, 2007, 19:31 UTC
Message-ID: <7vzlwe1jeb.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20071213180347.GE1224@artemis.madism.org>

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.

Show 6 quoted lines

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