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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Jeff King <peff@xxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]