Re: [PATCH v4 08/21] checkout-index: there are only two possible line terminations

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 15, 2016, 23:31 UTC
Message-ID: <xmqq8u3qfmix.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqq1t9ih72f.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 23 quoted lines

> Jeff King <peff@peff.net> writes:
>
>> On Thu, Jan 14, 2016 at 03:58:23PM -0800, Junio C Hamano wrote:
>>
>>> @@ -144,10 +145,7 @@ static int option_parse_u(const struct option *opt,
>>>  static int option_parse_z(const struct option *opt,
>>>  			  const char *arg, int unset)
>>>  {
>>> -	if (unset)
>>> -		line_termination = '\n';
>>> -	else
>>> -		line_termination = 0;
>>> +	nul_term_line = !unset;
>>>  	return 0;
>>>  }
>>
>> Is it worth doing this on top?
>
> Yes.
>
> To be bluntly honest, the above callback has no right to exist.  I
> should have turned it from OPTION_CALLBACK to OPT_BOOL from the
> start.

Having said that, I'd leave this change out of my tree for now, until the patches 1-9, which I'll split out from the rest of the series into a separate topic, moves out of the way.

I have this suspicion that "checkout-index" inherited the peculiar stance "update-index" took about the command line arguments and options (e.g. "update-index A --add B" and "update-index --add A B" do different things for path "A").

As far as I can see, there is no reason why "-u", just like "-z" that you noticed, has to be processed immediately as it is read before processing the remainder of the command line. And I think the change you suggested to stop using a callback for "-z" is better done with the same patch as the one that fixes "-u", whose log message would probably be:

    checkout-index: simplify option parsing

    For some reason, the command line parser for "-u" and "-z"
    options tried to make the effect of the options as they are
    processed by using OPTION_CALLBACK, but there is no reason
    to do so.  Just flip the bit to remember that we have seen
    these options while parsing the command line, and use them
    after we are done parsing, just like all other options.

or something.