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

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

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 branch.c | 48 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/branch.c b/branch.c
index eccdaf9..d8facf7 100644
--- a/branch.c
+++ b/branch.c
@@ -74,25 +74,43 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 		strbuf_addf(&key, "branch.%s.rebase", local);
 		git_config_set(key.buf, "true");
 	}
+	strbuf_release(&key);
 
 	if (flag & BRANCH_CONFIG_VERBOSE) {
-		strbuf_reset(&key);
-
-		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
+					printf("Branch %s set up to track local branch %s by rebasing.\n",
+					       local, shortname);
+			} else {
+				if (origin)
+					printf("Branch %s set up to track remote ref %s by rebasing.\n",
+					       local, remote);
+				else
+					printf("Branch %s set up to track local ref %s by rebasing.\n",
+					       local, remote);
+			}
+		} else {
+			if (remote_is_branch) {
+				if (origin)
+					printf("Branch %s set up to track remote branch %s from %s.\n",
+					       local, shortname, origin);
+				else
+					printf("Branch %s set up to track local branch %s.\n",
+					       local, shortname);
+			} else {
+				if (origin)
+					printf("Branch %s set up to track remote ref %s.\n",
+					       local, remote);
+				else
+					printf("Branch %s set up to track local ref %s.\n",
+					       local, remote);
+			}
 		}
-		else
-			strbuf_addf(&key, " ref %s", remote);
-		printf("Branch %s set up to track %s%s.\n",
-		       local, key.buf,
-		       rebasing ? " by rebasing" : "");
 	}
-	strbuf_release(&key);
 }
 
 /*
-- 
1.7.10.2.549.g9354186

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

[Jonathan Nieder]

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
[...]
> -		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(_(...

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

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 More compact code compared to the lasst version while maintaining
 -Wformat's effectiveness.

 branch.c |   38 +++++++++++++++++++++++---------------
 1 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/branch.c b/branch.c
index eccdaf9..2bef1e7 100644
--- a/branch.c
+++ b/branch.c
@@ -74,25 +74,33 @@ void install_branch_config(int flag, const char *local, const char *origin, cons
 		strbuf_addf(&key, "branch.%s.rebase", local);
 		git_config_set(key.buf, "true");
 	}
+	strbuf_release(&key);
 
 	if (flag & BRANCH_CONFIG_VERBOSE) {
-		strbuf_reset(&key);
-
-		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 (remote_is_branch && origin)
+			printf(rebasing ?
+			       "Branch %s set up to track remote branch %s from %s by rebasing.\n" :
+			       "Branch %s set up to track remote branch %s from %s.\n",
+			       local, shortname, origin);
+		else if (remote_is_branch && !origin)
+			printf(rebasing ?
+			       "Branch %s set up to track local branch %s by rebasing.\n" :
+			       "Branch %s set up to track local branch %s.\n",
+			       local, shortname);
+		else if (!remote_is_branch && origin)
+			printf(rebasing ?
+			       "Branch %s set up to track remote ref %s by rebasing.\n" :
+			       "Branch %s set up to track remote ref %s.\n",
+			       local, remote);
+		else if (!remote_is_branch && !origin)
+			printf(rebasing ?
+			       "Branch %s set up to track local ref %s by rebasing.\n" :
+			       "Branch %s set up to track local ref %s.\n",
+			       local, remote);
 		else
-			strbuf_addf(&key, " ref %s", remote);
-		printf("Branch %s set up to track %s%s.\n",
-		       local, key.buf,
-		       rebasing ? " by rebasing" : "");
+			die("BUG: impossible combination of %d and %p",
+			    remote_is_branch, origin);
 	}
-	strbuf_release(&key);
 }
 
 /*
-- 
1.7.8

[PATCH 2/6] reflog: remove i18n legos in pruning message

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Jonathan's version

 builtin/reflog.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 062d7da..b3c9e27 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -330,8 +330,10 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 		printf("keep %s", message);
 	return 0;
  prune:
-	if (!cb->newlog || cb->cmd->verbose)
-		printf("%sprune %s", cb->newlog ? "" : "would ", message);
+	if (!cb->newlog)
+		printf("would prune %s", message);
+	else if (cb->cmd->verbose)
+		printf("prune %s", message);
 	return 0;
 }
 
-- 
1.7.8

[PATCH 3/6] merge-recursive: remove i18n legos in conflict messages

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 The two sentence strings are broken down to one sentence each. Less work for
 translators when these are marked i18n.

 merge-recursive.c |   60 ++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index 680937c..1f16e04 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1018,9 +1018,28 @@ static void handle_change_delete(struct merge_options *o,
 				 const unsigned char *o_sha, int o_mode,
 				 const unsigned char *a_sha, int a_mode,
 				 const unsigned char *b_sha, int b_mode,
-				 const char *change, const char *change_past)
+				 const char *change)
 {
 	char *renamed = NULL;
+	struct strbuf sb = STRBUF_INIT;
+	int idx;
+
+	const char *msg[] = {
+		"CONFLICT (rename/delete): %s deleted in %s and renamed in %s.",
+		"CONFLICT (modify/delete): %s deleted in %s and modified in %s.",
+	};
+	const char *renamed_msg[] = {
+		"CONFLICT (rename/delete): %s deleted in %s and renamed in %s.",
+		"CONFLICT (modify/delete): %s deleted in %s and modified in %s.",
+	};
+
+	if (!strcmp(change, "rename"))
+		idx = 0;
+	else if (!strcmp(change, "modify"))
+		idx = 1;
+	else
+		die("BUG: unsupport action %s", change);
+
 	if (dir_in_way(path, !o->call_depth)) {
 		renamed = unique_path(o, path, a_sha ? o->branch1 : o->branch2);
 	}
@@ -1034,22 +1053,30 @@ static void handle_change_delete(struct merge_options *o,
 		remove_file_from_cache(path);
 		update_file(o, 0, o_sha, o_mode, renamed ? renamed : path);
 	} else if (!a_sha) {
-		output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
-		       "and %s in %s. Version %s of %s left in tree%s%s.",
-		       change, path, o->branch1,
-		       change_past, o->branch2, o->branch2, path,
-		       NULL == renamed ? "" : " at ",
-		       NULL == renamed ? "" : renamed);
+		if (renamed) {
+			strbuf_addf(&sb, renamed_msg[idx], path, o->branch1, o->branch2);
+			strbuf_addf(&sb, " Version %s of %s left in tree at %s.",
+				    o->branch2, path, renamed);
+		} else {
+			strbuf_addf(&sb, msg[idx], path, o->branch1, o->branch2);
+			strbuf_addf(&sb, " Version %s of %s left in tree.",
+				    o->branch2, path);
+		}
+		output(o, 1, "%s", sb.buf);
+
 		update_file(o, 0, b_sha, b_mode, renamed ? renamed : path);
 	} else {
-		output(o, 1, "CONFLICT (%s/delete): %s deleted in %s "
-		       "and %s in %s. Version %s of %s left in tree%s%s.",
-		       change, path, o->branch2,
-		       change_past, o->branch1, o->branch1, path,
-		       NULL == renamed ? "" : " at ",
-		       NULL == renamed ? "" : renamed);
-		if (renamed)
+		if (renamed) {
+			strbuf_addf(&sb, renamed_msg[idx], path, o->branch2, o->branch1);
+			strbuf_addf(&sb, " Version %s of %s left in tree at %s.",
+				    o->branch2, path, renamed);
 			update_file(o, 0, a_sha, a_mode, renamed);
+		} else {
+			strbuf_addf(&sb, msg[idx], path, o->branch2, o->branch1);
+			strbuf_addf(&sb, " Version %s of %s left in tree.",
+				    o->branch2, path);
+		}
+		output(o, 1, "%s", sb.buf);
 		/*
 		 * No need to call update_file() on path when !renamed, since
 		 * that would needlessly touch path.  We could call
@@ -1058,6 +1085,7 @@ static void handle_change_delete(struct merge_options *o,
 		 */
 	}
 	free(renamed);
+	strbuf_release(&sb);
 }
 
 static void conflict_rename_delete(struct merge_options *o,
@@ -1085,7 +1113,7 @@ static void conflict_rename_delete(struct merge_options *o,
 			     orig->sha1, orig->mode,
 			     a_sha, a_mode,
 			     b_sha, b_mode,
-			     "rename", "renamed");
+			     "rename");
 
 	if (o->call_depth) {
 		remove_file_from_cache(dest->path);
@@ -1568,7 +1596,7 @@ static void handle_modify_delete(struct merge_options *o,
 			     o_sha, o_mode,
 			     a_sha, a_mode,
 			     b_sha, b_mode,
-			     "modify", "modified");
+			     "modify");
 }
 
 static int merge_content(struct merge_options *o,
-- 
1.7.8

[PATCH 4/6] notes-merge: remove i18n legos in merge result message

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 notes-merge.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 74aa77c..29c6411 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -524,8 +524,10 @@ static int merge_from_diffs(struct notes_merge_options *o,
 	free(changes);
 
 	if (o->verbosity >= 4)
-		printf("Merge result: %i unmerged notes and a %s notes tree\n",
-			conflicts, t->dirty ? "dirty" : "clean");
+		printf(t->dirty ?
+		       "Merge result: %i unmerged notes and a dirty notes tree\n" :
+		       "Merge result: %i unmerged notes and a clean notes tree\n",
+		       conflicts);
 
 	return conflicts ? -1 : 1;
 }
-- 
1.7.8

[PATCH 5/6] rerere: remove i18n legos in result message

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 rerere.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/rerere.c b/rerere.c
index dcb525a..da18fc3 100644
--- a/rerere.c
+++ b/rerere.c
@@ -544,13 +544,13 @@ static int do_plain_rerere(struct string_list *rr, int fd)
 
 		if (has_rerere_resolution(name)) {
 			if (!merge(name, path)) {
-				if (rerere_autoupdate)
+				const char *msg;
+				if (rerere_autoupdate) {
 					string_list_insert(&update, path);
-				fprintf(stderr,
-					"%s '%s' using previous resolution.\n",
-					rerere_autoupdate
-					? "Staged" : "Resolved",
-					path);
+					msg = "Staged '%s' using previous resolution.\n";
+				} else
+					msg = "Resolved '%s' using previous resolution.\n";
+				fprintf(stderr, msg, path);
 				goto mark_resolved;
 			}
 		}
-- 
1.7.8

[PATCH 6/6] unpack-trees: remove i18n legos in unpack's porcelain error messages

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 unpack-trees.c |   55 +++++++++++++++++++++++++++++++++----------------------
 1 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index ad40109..41c5714 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -53,35 +53,46 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
 void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
 				  const char *cmd)
 {
-	int i;
+	int i, idx;
 	const char **msgs = opts->msgs;
-	const char *msg;
-	char *tmp;
-	const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
-	if (advice_commit_before_merge)
-		msg = "Your local changes to the following files would be overwritten by %s:\n%%s"
-			"Please, commit your changes or stash them before you can %s.";
+	struct strbuf msg = STRBUF_INIT;
+	const char *overwrite_advice[] = {
+		"Please, commit your changes or stash them before you can merge.",
+		"Please, commit your changes or stash them before you can switch branches."
+	};
+	const char *remove_untracked_advice[] = {
+		"Please move or remove them before you can merge.",
+		"Please move or remove them before you can switch branches."
+	};
+
+	if (!strcmp(cmd, "merge"))
+		idx = 0;
+	else if (!strcmp(cmd, "checkout"))
+		idx = 1;
 	else
-		msg = "Your local changes to the following files would be overwritten by %s:\n%%s";
-	tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);
-	sprintf(tmp, msg, cmd, cmd2);
-	msgs[ERROR_WOULD_OVERWRITE] = tmp;
-	msgs[ERROR_NOT_UPTODATE_FILE] = tmp;
+		die("BUG: unsupported command %s", cmd);
+
+	strbuf_addf(&msg, "Your local changes to the following files "
+		    "would be overwritten by %s:\n%%s", cmd);
+	if (advice_commit_before_merge)
+		strbuf_addstr(&msg, overwrite_advice[idx]);
+	msgs[ERROR_WOULD_OVERWRITE] = strbuf_detach(&msg, NULL);
+	msgs[ERROR_NOT_UPTODATE_FILE] = msgs[ERROR_WOULD_OVERWRITE];
 
 	msgs[ERROR_NOT_UPTODATE_DIR] =
 		"Updating the following directories would lose untracked files in it:\n%s";
 
+	strbuf_addf(&msg, "The following untracked working tree "
+		    "files would be removed by %s:\n%%s", cmd);
 	if (advice_commit_before_merge)
-		msg = "The following untracked working tree files would be %s by %s:\n%%s"
-			"Please move or remove them before you can %s.";
-	else
-		msg = "The following untracked working tree files would be %s by %s:\n%%s";
-	tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4);
-	sprintf(tmp, msg, "removed", cmd, cmd2);
-	msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp;
-	tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("overwritten") + strlen(cmd2) - 4);
-	sprintf(tmp, msg, "overwritten", cmd, cmd2);
-	msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp;
+		strbuf_addstr(&msg, remove_untracked_advice[idx]);
+	msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = strbuf_detach(&msg, NULL);
+
+	strbuf_addf(&msg, "The following untracked working tree "
+		    "files would be overwritten by %s:\n%%s", cmd);
+	if (advice_commit_before_merge)
+		strbuf_addstr(&msg, remove_untracked_advice[idx]);
+	msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = strbuf_detach(&msg, NULL);
 
 	/*
 	 * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we
-- 
1.7.8

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

[Junio C Hamano]

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

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  More compact code compared to the lasst version while maintaining
>  -Wformat's effectiveness.

I'd hesitate to burden translators with new set of strings this late
in the cycle.  The changes look reasonable, though.  Let's merge
this early in the next cycle and mark the strings for translation.

Thanks.