{"thread":{"id":"27199","subject":"[PATCH/RFC] inconsistent error messages for translation","startedAt":"2011-04-27T12:22:50Z","lastAt":"2011-04-27T17:17:30Z","messageCount":2,"participants":["=?UTF-8?q?Motiejus=20Jak=C5=A1tys?=","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"166513","messageId":"20110427122250.GA10919@jakstys.lt","threadId":"27199","inReplyTo":null,"subject":"[PATCH/RFC] inconsistent error messages for translation","fromName":"=?UTF-8?q?Motiejus=20Jak=C5=A1tys?=","fromEmail":"desired.mta@gmail.com","sentAt":"2011-04-27T12:22:50Z","receivedAt":"2011-04-27T12:22:50Z","isPatch":true,"sender":{"key":"desired.mta@gmail.com","avatar":"https://gravatar.com/avatar/5d7d88e7b672acebe3472e92baccb5f933fbd282b6131e22e32d96e5818b5b38?d=mp&s=160"},"body":"There are lots of variants of the same message:\n\nmsgid \"cannot stat '%s'\"\nmsgid \"failed to stat '%s'\"\nmsgid \"failed to stat %s\\n\"\nmsgid \"Could not stat '%s'\"\n\nThis patch makes them all \"Could not stat %s\" and \"Could not stat '%s'\\n\".\nThat makes .po file shorter.\n\nAlso same trivial fix:\n-\t\treturn error(_(\"path '%s' does not have all three versions\"),\n+\t\treturn error(_(\"path '%s' does not have all 3 versions\"),\n\nSigned-off-by: Motiejus Jakštys <desired.mta@gmail.com>\n---\n\nDRY:\n\n#: builtin/fetch.c:282\nsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : '-',\n    TRANSPORT_SUMMARY_WIDTH, _(\"[tag update]\"), REFCOL_WIDTH, remote,\n    pretty_ref, r ? _(\"  (unable to update local ref)\") : \"\");\n\n#: builtin/fetch.c:338\nsprintf(display, \"%c %-*s %-*s -> %s  (%s)\", r ? '!' : '+',\n    TRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n    pretty_ref,\n    r ? _(\"unable to update local ref\") : _(\"forced update\"));\n\nIt produces\n#: builtin/fetch.c:282 builtin/fetch.c:307 builtin/fetch.c:323\nmsgid \"  (unable to update local ref)\"\n#: builtin/fetch.c:338\nmsgid \"unable to update local ref\"\n\nI would like to have one string to translate instead of two.\n\nHow I would solve this:\n// or similar, unsure if this will work\nchar *without_brackets = _(\"unable to update local ref\");\nchar *with_brackets;\nsnprintf(with_brackets, 20, \"  (%s)\", trans);\n\n// -- code --\n    pretty_ref, r ? with_brackets : \"\");\n\nIt introduces 2 more variables. Is there a more elegant way?\n\nMotiejus\n\n builtin/checkout.c |    2 +-\n builtin/grep.c     |    2 +-\n builtin/init-db.c  |   24 ++++++++++++------------\n 3 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex eece5d6..417f03d 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -118,7 +118,7 @@ static int check_all_stages(struct cache_entry *ce, int pos)\n \t    ce_stage(active_cache[pos+1]) != 2 ||\n \t    strcmp(active_cache[pos+2]->name, ce->name) ||\n \t    ce_stage(active_cache[pos+2]) != 3)\n-\t\treturn error(_(\"path '%s' does not have all three versions\"),\n+\t\treturn error(_(\"path '%s' does not have all 3 versions\"),\n \t\t\t     ce->name);\n \treturn 0;\n }\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 10a1f65..24d19b8 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -413,7 +413,7 @@ static void *load_file(const char *filename, size_t *sz)\n \tif (lstat(filename, &st) < 0) {\n \terr_ret:\n \t\tif (errno != ENOENT)\n-\t\t\terror(_(\"'%s': %s\"), filename, strerror(errno));\n+\t\t\terror(\"'%s': %s\", filename, strerror(errno));\n \t\treturn NULL;\n \t}\n \tif (!S_ISREG(st.st_mode))\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex b7370d9..f1bee61 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -64,20 +64,20 @@ static void copy_templates_1(char *path, int baselen,\n \t\tmemcpy(template + template_baselen, de->d_name, namelen+1);\n \t\tif (lstat(path, &st_git)) {\n \t\t\tif (errno != ENOENT)\n-\t\t\t\tdie_errno(_(\"cannot stat '%s'\"), path);\n+\t\t\t\tdie_errno(_(\"Could not stat '%s'\"), path);\n \t\t}\n \t\telse\n \t\t\texists = 1;\n \n \t\tif (lstat(template, &st_template))\n-\t\t\tdie_errno(_(\"cannot stat template '%s'\"), template);\n+\t\t\tdie_errno(_(\"Could not stat template '%s'\"), template);\n \n \t\tif (S_ISDIR(st_template.st_mode)) {\n \t\t\tDIR *subdir = opendir(template);\n \t\t\tint baselen_sub = baselen + namelen;\n \t\t\tint template_baselen_sub = template_baselen + namelen;\n \t\t\tif (!subdir)\n-\t\t\t\tdie_errno(_(\"cannot opendir '%s'\"), template);\n+\t\t\t\tdie_errno(_(\"Could not opendir '%s'\"), template);\n \t\t\tpath[baselen_sub++] =\n \t\t\t\ttemplate[template_baselen_sub++] = '/';\n \t\t\tpath[baselen_sub] =\n@@ -94,16 +94,16 @@ static void copy_templates_1(char *path, int baselen,\n \t\t\tint len;\n \t\t\tlen = readlink(template, lnk, sizeof(lnk));\n \t\t\tif (len < 0)\n-\t\t\t\tdie_errno(_(\"cannot readlink '%s'\"), template);\n+\t\t\t\tdie_errno(_(\"Could not readlink '%s'\"), template);\n \t\t\tif (sizeof(lnk) <= len)\n \t\t\t\tdie(_(\"insanely long symlink %s\"), template);\n \t\t\tlnk[len] = 0;\n \t\t\tif (symlink(lnk, path))\n-\t\t\t\tdie_errno(_(\"cannot symlink '%s' '%s'\"), lnk, path);\n+\t\t\t\tdie_errno(_(\"Could not symlink '%s' '%s'\"), lnk, path);\n \t\t}\n \t\telse if (S_ISREG(st_template.st_mode)) {\n \t\t\tif (copy_file(path, template, st_template.st_mode))\n-\t\t\t\tdie_errno(_(\"cannot copy '%s' to '%s'\"), template,\n+\t\t\t\tdie_errno(_(\"Could not copy '%s' to '%s'\"), template,\n \t\t\t\t\t  path);\n \t\t}\n \t\telse\n@@ -437,7 +437,7 @@ static int guess_repository_type(const char *git_dir)\n \tif (!strcmp(\".\", git_dir))\n \t\treturn 1;\n \tif (!getcwd(cwd, sizeof(cwd)))\n-\t\tdie_errno(_(\"cannot tell cwd\"));\n+\t\tdie_errno(_(\"Could not tell cwd\"));\n \tif (!strcmp(git_dir, cwd))\n \t\treturn 1;\n \t/*\n@@ -518,18 +518,18 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\t\terrno = EEXIST;\n \t\t\t\t\t/* fallthru */\n \t\t\t\tcase -1:\n-\t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n+\t\t\t\t\tdie_errno(_(\"Could not mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\n \t\t\t\tdefault:\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t\tshared_repository = saved;\n \t\t\t\tif (mkdir(argv[0], 0777) < 0)\n-\t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n+\t\t\t\t\tdie_errno(_(\"Could not mkdir %s\"), argv[0]);\n \t\t\t\tmkdir_tried = 1;\n \t\t\t\tgoto retry;\n \t\t\t}\n-\t\t\tdie_errno(_(\"cannot chdir to %s\"), argv[0]);\n+\t\t\tdie_errno(_(\"Could not chdir to %s\"), argv[0]);\n \t\t}\n \t} else if (0 < argc) {\n \t\tusage(init_db_usage[0]);\n@@ -575,14 +575,14 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tif (!git_work_tree_cfg) {\n \t\t\tgit_work_tree_cfg = xcalloc(PATH_MAX, 1);\n \t\t\tif (!getcwd(git_work_tree_cfg, PATH_MAX))\n-\t\t\t\tdie_errno (_(\"Cannot access current working directory\"));\n+\t\t\t\tdie_errno (_(\"Could not access current working directory\"));\n \t\t}\n \t\tif (work_tree)\n \t\t\tset_git_work_tree(real_path(work_tree));\n \t\telse\n \t\t\tset_git_work_tree(git_work_tree_cfg);\n \t\tif (access(get_git_work_tree(), X_OK))\n-\t\t\tdie_errno (_(\"Cannot access work tree '%s'\"),\n+\t\t\tdie_errno (_(\"Could not access work tree '%s'\"),\n \t\t\t\t   get_git_work_tree());\n \t}\n \telse {\n-- \n1.7.2.5\n"},{"id":"166530","messageId":"7vr58n8vh1.fsf@alter.siamese.dyndns.org","threadId":"27199","inReplyTo":"20110427122250.GA10919@jakstys.lt","subject":"Re: [PATCH/RFC] inconsistent error messages for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-27T17:17:30Z","receivedAt":"2011-04-27T17:17:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Motiejus Jakštys <desired.mta@gmail.com> writes:\n\n> There are lots of variants of the same message:\n>\n> msgid \"cannot stat '%s'\"\n> msgid \"failed to stat '%s'\"\n> msgid \"failed to stat %s\\n\"\n> msgid \"Could not stat '%s'\"\n\nI am not sure what to do with the trailing LF (it may be a bug in the\nmessage written without being aware that die/warn will give their own LF\nat the end), but the first one (\"cannot $verb '$name'\") is preferred.\n\n> Also same trivial fix:\n> -\t\treturn error(_(\"path '%s' does not have all three versions\"),\n> +\t\treturn error(_(\"path '%s' does not have all 3 versions\"),\n\nI would not call this a \"fix\", though.  What problem does it solve?\n\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 10a1f65..24d19b8 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -413,7 +413,7 @@ static void *load_file(const char *filename, size_t *sz)\n>  \tif (lstat(filename, &st) < 0) {\n>  \terr_ret:\n>  \t\tif (errno != ENOENT)\n> -\t\t\terror(_(\"'%s': %s\"), filename, strerror(errno));\n> +\t\t\terror(\"'%s': %s\", filename, strerror(errno));\n>  \t\treturn NULL;\n>  \t}\n>  \tif (!S_ISREG(st.st_mode))\n\nThis hunk is a fix for mismarked message and is unrelated to the error\nmessage unification, no?  I prefer to have only this part as a separate\npatch.\n"}]}