Re: [PATCH v9 19/20] branch: use ref-filter printing APIs

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 27, 2016, 21:17 UTC
Message-ID: <xmqqinq56phu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161227162357.28212-20-Karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

Show 14 quoted lines

>  static char branch_colors[][COLOR_MAXLEN] = {
> -	GIT_COLOR_RESET,
> -	GIT_COLOR_NORMAL,	/* PLAIN */
> -	GIT_COLOR_RED,		/* REMOTE */
> -	GIT_COLOR_NORMAL,	/* LOCAL */
> -	GIT_COLOR_GREEN,	/* CURRENT */
> -	GIT_COLOR_BLUE,		/* UPSTREAM */
> +	"%(color:reset)",
> +	"%(color:reset)",	/* PLAIN */
> +	"%(color:red)",		/* REMOTE */
> +	"%(color:reset)",	/* LOCAL */
> +	"%(color:green)",	/* CURRENT */
> +	"%(color:blue)",	/* UPSTREAM */
>  };

The contents of this table is no longer tied to COLOR_MAXLEN. The above entries used by default happen to be shorter, but it is misleading.

	static const char *branch_colors[] = {
	"%(color:reset)",
	...
	};

perhaps?

More importantly, does this re-definition of the branch_colors[] work with custom colors filled in git_branch_config() by calling e.g. color_parse("red", branch_colors[BRANCH_COLOR_REMOTE]), where "red" and "remote" come from an existing configuration file?

	[color "branch"]
		remote = red

It obviously would not work if you changed the type of branch_colors[] because the color_parse() wants the caller to have allocated space of COLOR_MAXLEN.

But if filling these ANSI escape sequence into the format works OK, then doesn't it tell us that we do not need to have this change to use "%(color:reset)" etc. as the new default values?