Re: [PATCH 1/5 v4] diff: parse detached options like -S foo

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 2, 2010, 19:47 UTC
Message-ID: <20100802194724.GC2180@burratino>
In-Reply-To: <7v62ztnesc.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

> Matthieu Moy <Matthieu.Moy@imag.fr> writes:

Show 6 quoted lines

>> +	if (arg[2] != '\0') {
>> +		*optarg = arg + strlen("-c");
>
> Just a style thing, but I think "arg + 2" is much easier to read in this
> particular case, as it won't risk tempting readers to go "Huh?  What does
> that 'c' mean"?

Yeah, especially after the "if (arg[2] != ...)". Thanks for a sanity check.

> Do we have an option that can take a zero-length string as its value and
> do something meaningful?  I don't think of any offhand ("log -S'' -p" is
> not it---it may be meaningful but it is not useful), but this code would
> start giving "-p" instead of "" to the option in such a case.

 -l<num>:	"" is not a number.

 -S<artifact>:
		Could "" come up if a user tries to find a commit
		"Adding/removing string:" without typing a string to look
		for in gitk?  No, gitk protects against that.
		How about creating a view with an empty "Search string:"?
		No, that is protected against, too.

		Currently "git log -S" does not seem to do anything
		meaningful, anyway.

 -O<file>:	"" is not a file name.