Re: [PATCH 2/5] builtin/config.c: support `--type=<type>` as preferred alias for `--type`

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 26, 2018, 05:25 UTC
Message-ID: <xmqq7eou35ev.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <1bc42a611089a2552b1d8bc83ff229be0a5136ff.1524716420.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=<type>` as preferred alias for `--type`

I'd retitle while queuing, as the last 'type' is a placeholder for concrete types like <type> above.

Show 5 quoted lines

> +static int option_parse_type(const struct option *opt, const char *arg,
> +			     int unset)
> +{
> +	int new_type;
> +	int *to_type;

Splitting these into two lines (unlike placing on a single same line, which was how the previous round was queued) like this is good.

Show 7 quoted lines

> +...
> +	new_type = opt->defval;
> +	if (!new_type) {
> +...
> +	}
> +
> +	*to_type = opt->value;

But this is wrong, no? You meant opt->value points at an integer variable that receives the type we discover by parsing, i.e.

	to_type = opt->value;