Re: [PATCH] Add column layout skeleton and git-column

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 16, 2012, 16:05 UTC
Message-ID: <7v4nsjy85r.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1334582784-25723-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

Show 33 quoted lines

> +static int parse_config(unsigned int *colopts, const char *value)
> +{
> +	const char *sep = " ,";
> +	int group_set = 0;
> +
> +	while (*value) {
> +		int len = strcspn(value, sep);
> +		if (len) {
> +			if (parse_option(value, len, colopts, &group_set))
> +				return -1;
> +
> +			value += len;
> +		}
> +		value += strspn(value, sep);
> +	}
> +	/*
> +	 * Setting layout implies "always" if neither always, never
> +	 * nor auto is specified.
> +	 *
> +	 * Current COL_ENABLE() value is disregarded. This means if
> +	 * you set column.ui = auto and pass --column=row, then "auto"
> +	 * will become "always".
> +	 */
> +	if ((group_set & LAYOUT_SET) && !(group_set & ENABLE_SET))
> +		*colopts = (*colopts & ~COL_ENABLE_MASK) | COL_ENABLED;
> +	return 0;
> +}
> +
> +static int column_config(const char *var, const char *value,
> +			 const char *key, unsigned int *colopts)
> +{
> +	if (value && parse_config(colopts, value))
> +		return error("invalid column.%s mode %s", key, value);

If column.* can never be a boolean, this code shouldn't silently ignore

	[column]
        	ui

but explicitly error out. See git_config_string() for an example if you want to go this route.

On the other hand, it might make sense to equate a "true" with "a sane default" (i.e. the user declares that s/he trusts whatever default the Git implementation deems sane), in which case

	if (!value)
		value = "auto,whatever,column,default,mode";
	if (parse_config(colopts, value))
        	return error("invalid column.%s mode...", ...);

might be a better way to go. If you take this route, a single-token column.* becomes a variable that takes boolean and more, so the existing `never` has to gain `no`, `false`, etc. as synonyms, and `true`, `1`, etc. needs to be interpreted the same as `auto` for consistency.