Re: [PATCH 1/6] Remove i18n legos in notifying new branch tracking setup

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 31, 2012, 14:00 UTC
Message-ID: <20120531140024.GC10523@burratino>
In-Reply-To: <1338469482-30936-2-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy wrote:

> Not sure if it's the right way to fix these though. For example, while
> 1/1 looks very good from i18n perspective, code-wise it's quite ugly.
> Grouping format strings in array also prevents gcc from checking
> correct parameters, I think.

[...]

> --- a/branch.c
> +++ b/branch.c
> @@ -74,25 +74,43 @@ void install_branch_config(int flag, const char *local, const char *origin, cons

[...]

Show 13 quoted lines

> -		strbuf_addstr(&key, origin ? "remote" : "local");
> -
> -		/* Are we tracking a proper "branch"? */
> -		if (remote_is_branch) {
> -			strbuf_addf(&key, " branch %s", shortname);
> -			if (origin)
> -				strbuf_addf(&key, " from %s", origin);
> +		if (rebasing) {
> +			if (remote_is_branch) {
> +				if (origin)
> +					printf("Branch %s set up to track remote branch %s from %s by rebasing.\n",
> +					       local, shortname, origin);
> +				else

[...]

> +			} else {
> +				if (origin)

[...]

> +			}
> +		} else {
> +			if (remote_is_branch) {

[...]

I think a table-driven version of this switchboard would be much easier to read, even if it would hurt gcc's -Wformat checking. If the -Wformat safety is too precious to lose, would something like the following work?

	switch (tracking_msg_flags) {
	case REBASING | REMOTE_IS_BRANCH | ORIGIN:
		printf(_("Branch %s set up to track remote branch %s ..."),
			...
		break;
	case REBASING | REMOTE_IS_BRANCH:
		printf(_(...