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

Junio C Hamano [off-list ref] writes:

Jeff King [off-list ref] writes:

quoted

On Thu, Jan 14, 2016 at 03:58:23PM -0800, Junio C Hamano wrote:

quoted

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