[PATCH/RFC] inconsistent error messages for translation

[PATCH/RFC] inconsistent error messages for translation

[=?UTF-8?q?Motiejus=20Jak=C5=A1tys?=]

There are lots of variants of the same message:

msgid "cannot stat '%s'"
msgid "failed to stat '%s'"
msgid "failed to stat %s\n"
msgid "Could not stat '%s'"

This patch makes them all "Could not stat %s" and "Could not stat '%s'\n".
That makes .po file shorter.

Also same trivial fix:
-		return error(_("path '%s' does not have all three versions"),
+		return error(_("path '%s' does not have all 3 versions"),

Signed-off-by: Motiejus Jakštys <desired.mta@gmail.com>
---

DRY:

#: builtin/fetch.c:282
sprintf(display, "%c %-*s %-*s -> %s%s", r ? '!' : '-',
    TRANSPORT_SUMMARY_WIDTH, _("[tag update]"), REFCOL_WIDTH, remote,
    pretty_ref, r ? _("  (unable to update local ref)") : "");

#: builtin/fetch.c:338
sprintf(display, "%c %-*s %-*s -> %s  (%s)", r ? '!' : '+',
    TRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,
    pretty_ref,
    r ? _("unable to update local ref") : _("forced update"));

It produces
#: builtin/fetch.c:282 builtin/fetch.c:307 builtin/fetch.c:323
msgid "  (unable to update local ref)"
#: builtin/fetch.c:338
msgid "unable to update local ref"

I would like to have one string to translate instead of two.

How I would solve this:
// or similar, unsure if this will work
char *without_brackets = _("unable to update local ref");
char *with_brackets;
snprintf(with_brackets, 20, "  (%s)", trans);

// -- code --
    pretty_ref, r ? with_brackets : "");

It introduces 2 more variables. Is there a more elegant way?

Motiejus

 builtin/checkout.c |    2 +-
 builtin/grep.c     |    2 +-
 builtin/init-db.c  |   24 ++++++++++++------------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index eece5d6..417f03d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -118,7 +118,7 @@ static int check_all_stages(struct cache_entry *ce, int pos)
 	    ce_stage(active_cache[pos+1]) != 2 ||
 	    strcmp(active_cache[pos+2]->name, ce->name) ||
 	    ce_stage(active_cache[pos+2]) != 3)
-		return error(_("path '%s' does not have all three versions"),
+		return error(_("path '%s' does not have all 3 versions"),
 			     ce->name);
 	return 0;
 }
diff --git a/builtin/grep.c b/builtin/grep.c
index 10a1f65..24d19b8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -413,7 +413,7 @@ static void *load_file(const char *filename, size_t *sz)
 	if (lstat(filename, &st) < 0) {
 	err_ret:
 		if (errno != ENOENT)
-			error(_("'%s': %s"), filename, strerror(errno));
+			error("'%s': %s", filename, strerror(errno));
 		return NULL;
 	}
 	if (!S_ISREG(st.st_mode))
diff --git a/builtin/init-db.c b/builtin/init-db.c
index b7370d9..f1bee61 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -64,20 +64,20 @@ static void copy_templates_1(char *path, int baselen,
 		memcpy(template + template_baselen, de->d_name, namelen+1);
 		if (lstat(path, &st_git)) {
 			if (errno != ENOENT)
-				die_errno(_("cannot stat '%s'"), path);
+				die_errno(_("Could not stat '%s'"), path);
 		}
 		else
 			exists = 1;
 
 		if (lstat(template, &st_template))
-			die_errno(_("cannot stat template '%s'"), template);
+			die_errno(_("Could not stat template '%s'"), template);
 
 		if (S_ISDIR(st_template.st_mode)) {
 			DIR *subdir = opendir(template);
 			int baselen_sub = baselen + namelen;
 			int template_baselen_sub = template_baselen + namelen;
 			if (!subdir)
-				die_errno(_("cannot opendir '%s'"), template);
+				die_errno(_("Could not opendir '%s'"), template);
 			path[baselen_sub++] =
 				template[template_baselen_sub++] = '/';
 			path[baselen_sub] =
@@ -94,16 +94,16 @@ static void copy_templates_1(char *path, int baselen,
 			int len;
 			len = readlink(template, lnk, sizeof(lnk));
 			if (len < 0)
-				die_errno(_("cannot readlink '%s'"), template);
+				die_errno(_("Could not readlink '%s'"), template);
 			if (sizeof(lnk) <= len)
 				die(_("insanely long symlink %s"), template);
 			lnk[len] = 0;
 			if (symlink(lnk, path))
-				die_errno(_("cannot symlink '%s' '%s'"), lnk, path);
+				die_errno(_("Could not symlink '%s' '%s'"), lnk, path);
 		}
 		else if (S_ISREG(st_template.st_mode)) {
 			if (copy_file(path, template, st_template.st_mode))
-				die_errno(_("cannot copy '%s' to '%s'"), template,
+				die_errno(_("Could not copy '%s' to '%s'"), template,
 					  path);
 		}
 		else
@@ -437,7 +437,7 @@ static int guess_repository_type(const char *git_dir)
 	if (!strcmp(".", git_dir))
 		return 1;
 	if (!getcwd(cwd, sizeof(cwd)))
-		die_errno(_("cannot tell cwd"));
+		die_errno(_("Could not tell cwd"));
 	if (!strcmp(git_dir, cwd))
 		return 1;
 	/*
@@ -518,18 +518,18 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 					errno = EEXIST;
 					/* fallthru */
 				case -1:
-					die_errno(_("cannot mkdir %s"), argv[0]);
+					die_errno(_("Could not mkdir %s"), argv[0]);
 					break;
 				default:
 					break;
 				}
 				shared_repository = saved;
 				if (mkdir(argv[0], 0777) < 0)
-					die_errno(_("cannot mkdir %s"), argv[0]);
+					die_errno(_("Could not mkdir %s"), argv[0]);
 				mkdir_tried = 1;
 				goto retry;
 			}
-			die_errno(_("cannot chdir to %s"), argv[0]);
+			die_errno(_("Could not chdir to %s"), argv[0]);
 		}
 	} else if (0 < argc) {
 		usage(init_db_usage[0]);
@@ -575,14 +575,14 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 		if (!git_work_tree_cfg) {
 			git_work_tree_cfg = xcalloc(PATH_MAX, 1);
 			if (!getcwd(git_work_tree_cfg, PATH_MAX))
-				die_errno (_("Cannot access current working directory"));
+				die_errno (_("Could not access current working directory"));
 		}
 		if (work_tree)
 			set_git_work_tree(real_path(work_tree));
 		else
 			set_git_work_tree(git_work_tree_cfg);
 		if (access(get_git_work_tree(), X_OK))
-			die_errno (_("Cannot access work tree '%s'"),
+			die_errno (_("Could not access work tree '%s'"),
 				   get_git_work_tree());
 	}
 	else {
-- 
1.7.2.5

Re: [PATCH/RFC] inconsistent error messages for translation

[Junio C Hamano]

Motiejus Jakštys <desired.mta@gmail.com> writes:

> There are lots of variants of the same message:
>
> msgid "cannot stat '%s'"
> msgid "failed to stat '%s'"
> msgid "failed to stat %s\n"
> msgid "Could not stat '%s'"

I am not sure what to do with the trailing LF (it may be a bug in the
message written without being aware that die/warn will give their own LF
at the end), but the first one ("cannot $verb '$name'") is preferred.

> Also same trivial fix:
> -		return error(_("path '%s' does not have all three versions"),
> +		return error(_("path '%s' does not have all 3 versions"),

I would not call this a "fix", though.  What problem does it solve?

> diff --git a/builtin/grep.c b/builtin/grep.c
> index 10a1f65..24d19b8 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -413,7 +413,7 @@ static void *load_file(const char *filename, size_t *sz)
>  	if (lstat(filename, &st) < 0) {
>  	err_ret:
>  		if (errno != ENOENT)
> -			error(_("'%s': %s"), filename, strerror(errno));
> +			error("'%s': %s", filename, strerror(errno));
>  		return NULL;
>  	}
>  	if (!S_ISREG(st.st_mode))

This hunk is a fix for mismarked message and is unrelated to the error
message unification, no?  I prefer to have only this part as a separate
patch.