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, 21:22 UTC
Message-ID: <xmqq1t9ih72f.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160115200856.GC11301@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 15 quoted lines

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

Show 9 quoted lines

>
> -- >8 --
> Subject: [PATCH] checkout-index: simplify "-z" option parsing
>
> Now that we act as a simple bool, there's no need to use a
> custom callback.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Show 33 quoted lines

>  builtin/checkout-index.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
> index d8d7bd3..3b913d1 100644
> --- a/builtin/checkout-index.c
> +++ b/builtin/checkout-index.c
> @@ -142,13 +142,6 @@ static int option_parse_u(const struct option *opt,
>  	return 0;
>  }
>  
> -static int option_parse_z(const struct option *opt,
> -			  const char *arg, int unset)
> -{
> -	nul_term_line = !unset;
> -	return 0;
> -}
> -
>  static int option_parse_prefix(const struct option *opt,
>  			       const char *arg, int unset)
>  {
> @@ -192,9 +185,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
>  		{ OPTION_CALLBACK, 'u', "index", &newfd, NULL,
>  			N_("update stat information in the index file"),
>  			PARSE_OPT_NOARG, option_parse_u },
> -		{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
> -			N_("paths are separated with NUL character"),
> -			PARSE_OPT_NOARG, option_parse_z },
> +		OPT_BOOL('z', NULL, &nul_term_line,
> +			N_("paths are separated with NUL character")),
>  		OPT_BOOL(0, "stdin", &read_from_stdin,
>  			N_("read list of paths from the standard input")),
>  		OPT_BOOL(0, "temp", &to_tempfile,