From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mon, 02 Aug 2010 19:47:24 GMT
Subject: Re: [PATCH 1/5 v4] diff: parse detached options like -S foo
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:

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