[PATCH 1/1] Mark messages for translations
Reuse already translated messages if possible Do not translate messages aimed at developers of git Signed-off-by: Alexander Shopov --- git.c | 30 +++--- setup.c | 52 ++-- 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/git.c b/git.c index c870b9719..e66316ce5 100644 --- a/git.c +++ b/git.c @@ -5,11 +5,11 @@ #include "run-command.h" const char git_usage_string[] = - "git [--version] [--help] [-C ] [-c name=value]\n" - " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" - " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" - " [--git-dir=] [--work-tree=] [--namespace=]\n" - "[]"; + N_("git [--version] [--help] [-C ] [-c name=value]\n" + " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" + " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" + " [--git-dir=] [--work-tree=] [--namespace=]\n" + "[]"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -92,7 +92,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --git-dir.\n" ); + fprintf(stderr, _("No directory given for --git-dir.\n" )); usage(git_usage_string); } setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1); @@ -106,7 +106,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--namespace")) { if (*argc < 2) { - fprintf(stderr, "No namespace given for --namespace.\n" ); + fprintf(stderr, _("No namespace given for --namespace.\n" )); usage(git_usage_string); } setenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1); @@ -120,7 +120,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--work-tree")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --work-tree.\n" ); + fprintf(stderr, _("No directory given for --work-tree.\n" )); usage(git_usage_string); } setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1); @@ -134,7 +134,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--super-prefix")) { if (*argc < 2) { - fprintf(stderr, "No prefix given for --super-prefix.\n" ); + fprintf(stderr, _("No prefix given for --super-prefix.\n" )); usage(git_usage_string); } setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); @@ -156,7 +156,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-c")) { if (*argc < 2) { - fprintf(stderr, "-c expects a configuration string\n" ); + fprintf(stderr, _("-c expects a configuration string\n" )); usage(git_usage_string); } git_config_push_parameter((*argv)[1]); @@ -194,7 +194,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-C")) { if (*argc < 2) { - fprintf(stderr, "No directory given for -C.\n" ); + fprintf(stderr, _("No directory given for -C.\n" )); usage(git_usage_string); } if ((*argv)[1][0]) { @@ -209,7 +209,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) list_builtins(); exit(0); } else { - fprintf(stderr, "Unknown option: %s\n", cmd); + fprintf(stderr, _("Unknown opt
[PATCH 1/1] Mark messages for translations
Small changes in messages to fit the style and typography of rest Reuse already translated messages if possible Do not translate messages aimed at developers of git Fix unit tests depending on the original string Signed-off-by: Alexander Shopov --- git.c | 38 - setup.c| 62 +++--- t/t0002-gitfile.sh | 4 ++-- t/t0008-ignores.sh | 2 +- 4 files changed, 53 insertions(+), 53 deletions(-) diff --git a/git.c b/git.c index c870b9719..5ddcb75d4 100644 --- a/git.c +++ b/git.c @@ -5,11 +5,11 @@ #include "run-command.h" const char git_usage_string[] = - "git [--version] [--help] [-C ] [-c name=value]\n" - " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" - " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" - " [--git-dir=] [--work-tree=] [--namespace=]\n" - "[]"; + N_("git [--version] [--help] [-C ] [-c =]\n" + " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" + " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" + " [--git-dir=] [--work-tree=] [--namespace=]\n" + "[]"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -92,7 +92,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --git-dir.\n" ); + fprintf(stderr, _("no directory given for --git-dir\n" )); usage(git_usage_string); } setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1); @@ -106,7 +106,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--namespace")) { if (*argc < 2) { - fprintf(stderr, "No namespace given for --namespace.\n" ); + fprintf(stderr, _("no namespace given for --namespace\n" )); usage(git_usage_string); } setenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1); @@ -120,7 +120,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--work-tree")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --work-tree.\n" ); + fprintf(stderr, _("no directory given for --work-tree\n" )); usage(git_usage_string); } setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1); @@ -134,7 +134,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--super-prefix")) { if (*argc < 2) { - fprintf(stderr, "No prefix given for --super-prefix.\n" ); + fprintf(stderr, _("no prefix given for --super-prefix\n" )); usage(git_usage_string); } setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); @@ -156,7 +156,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-c")) { if (*argc < 2) { - fprintf(stderr, "-c expects a configuration string\n" ); + fprintf(stderr, _("-c expects a configuration string\n" )); usage(git_usage_string); } git_config_push_parameter((*argv)[1]); @@ -194,12 +194,12 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-C")) { if (*argc < 2) { - fprintf(stderr, "No directory given for -C.\n" ); + fprintf(stderr, _("no directory given for -C\n" )); usage(git_usage_string); } if ((*argv)[1][0]) { if (chdir((*argv)[1])) - die_errno("Cannot change to '%s'",
[PATCH 1/1] Mark messages for translations
Small changes in messages to fit the style and typography of rest Reuse already translated messages if possible Do not translate messages aimed at developers of git Fix unit tests depending on the original string Use `test_i18ngrep` for tests with translatable strings Change and verifyrest of tests via `make GETTEXT_POISON=1 test` Signed-off-by: Alexander Shopov --- git.c | 38 +- setup.c| 62 +- t/t0002-gitfile.sh | 4 +-- t/t0008-ignores.sh | 2 +- t/t1506-rev-parse-diagnosis.sh | 2 +- 5 files changed, 54 insertions(+), 54 deletions(-) diff --git a/git.c b/git.c index c870b9719..5ddcb75d4 100644 --- a/git.c +++ b/git.c @@ -5,11 +5,11 @@ #include "run-command.h" const char git_usage_string[] = - "git [--version] [--help] [-C ] [-c name=value]\n" - " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" - " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" - " [--git-dir=] [--work-tree=] [--namespace=]\n" - "[]"; + N_("git [--version] [--help] [-C ] [-c =]\n" + " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" + " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" + " [--git-dir=] [--work-tree=] [--namespace=]\n" + "[]"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -92,7 +92,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --git-dir.\n" ); + fprintf(stderr, _("no directory given for --git-dir\n" )); usage(git_usage_string); } setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1); @@ -106,7 +106,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--namespace")) { if (*argc < 2) { - fprintf(stderr, "No namespace given for --namespace.\n" ); + fprintf(stderr, _("no namespace given for --namespace\n" )); usage(git_usage_string); } setenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1); @@ -120,7 +120,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--work-tree")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --work-tree.\n" ); + fprintf(stderr, _("no directory given for --work-tree\n" )); usage(git_usage_string); } setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1); @@ -134,7 +134,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--super-prefix")) { if (*argc < 2) { - fprintf(stderr, "No prefix given for --super-prefix.\n" ); + fprintf(stderr, _("no prefix given for --super-prefix\n" )); usage(git_usage_string); } setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); @@ -156,7 +156,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-c")) { if (*argc < 2) { - fprintf(stderr, "-c expects a configuration string\n" ); + fprintf(stderr, _("-c expects a configuration string\n" )); usage(git_usage_string); } git_config_push_parameter((*argv)[1]); @@ -194,12 +194,12 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-C")) { if (*argc < 2) { - fprintf(stderr, "No directory given for -C.\n" ); + fprintf(stderr, _("no directory given for -C\n" )); usage(git_usage_string);
[PATCH 1/1] Mark messages for translations
Small changes in messages to fit the style and typography of rest. Reuse already translated messages if possible. Do not translate messages aimed at developers of git. Fix unit tests depending on the original string. Use `test_i18ngrep` for tests with translatable strings. Change and verify rest of tests via `make GETTEXT_POISON=1 test`. Signed-off-by: Alexander Shopov --- git.c | 38 +- setup.c| 62 +- t/t0002-gitfile.sh | 4 +-- t/t0008-ignores.sh | 2 +- t/t1506-rev-parse-diagnosis.sh | 2 +- 5 files changed, 54 insertions(+), 54 deletions(-) diff --git a/git.c b/git.c index c870b9719..5ddcb75d4 100644 --- a/git.c +++ b/git.c @@ -5,11 +5,11 @@ #include "run-command.h" const char git_usage_string[] = - "git [--version] [--help] [-C ] [-c name=value]\n" - " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" - " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" - " [--git-dir=] [--work-tree=] [--namespace=]\n" - "[]"; + N_("git [--version] [--help] [-C ] [-c =]\n" + " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" + " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" + " [--git-dir=] [--work-tree=] [--namespace=]\n" + "[]"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -92,7 +92,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --git-dir.\n" ); + fprintf(stderr, _("no directory given for --git-dir\n" )); usage(git_usage_string); } setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1); @@ -106,7 +106,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--namespace")) { if (*argc < 2) { - fprintf(stderr, "No namespace given for --namespace.\n" ); + fprintf(stderr, _("no namespace given for --namespace\n" )); usage(git_usage_string); } setenv(GIT_NAMESPACE_ENVIRONMENT, (*argv)[1], 1); @@ -120,7 +120,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--work-tree")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --work-tree.\n" ); + fprintf(stderr, _("no directory given for --work-tree\n" )); usage(git_usage_string); } setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1); @@ -134,7 +134,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--super-prefix")) { if (*argc < 2) { - fprintf(stderr, "No prefix given for --super-prefix.\n" ); + fprintf(stderr, _("no prefix given for --super-prefix\n" )); usage(git_usage_string); } setenv(GIT_SUPER_PREFIX_ENVIRONMENT, (*argv)[1], 1); @@ -156,7 +156,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-c")) { if (*argc < 2) { - fprintf(stderr, "-c expects a configuration string\n" ); + fprintf(stderr, _("-c expects a configuration string\n" )); usage(git_usage_string); } git_config_push_parameter((*argv)[1]); @@ -194,12 +194,12 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "-C")) { if (*argc < 2) { - fprintf(stderr, "No directory given for -C.\n" ); + fprintf(stderr, _("no directory given for -C\n" )); usage(git_usage_string);
Re: [PATCH 1/1] Mark messages for translations
Am 15.01.2018 um 06:44 schrieb Alexander Shopov: @@ -5,11 +5,11 @@ #include "run-command.h" const char git_usage_string[] = - "git [--version] [--help] [-C ] [-c name=value]\n" - " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" - " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" - " [--git-dir=] [--work-tree=] [--namespace=]\n" - "[]"; + N_("git [--version] [--help] [-C ] [-c name=value]\n" + " [--exec-path[=]] [--html-path] [--man-path] [--info-path]\n" + " [-p | --paginate | --no-pager] [--no-replace-objects] [--bare]\n" + " [--git-dir=] [--work-tree=] [--namespace=]\n" + "[]"); const char git_more_info_string[] = N_("'git help -a' and 'git help -g' list available subcommands and some\n" @@ -92,7 +92,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { if (*argc < 2) { - fprintf(stderr, "No directory given for --git-dir.\n" ); + fprintf(stderr, _("No directory given for --git-dir.\n" )); usage(git_usage_string); It is not obvious to me where git_usage_string is looked up in the message catalog. Should this not be usage(_(git_usage_string)); (here and in later instances)? It is used that way in builtin/help.c, for example. @@ -385,14 +385,14 @@ void setup_work_tree(void) return; if (work_tree_config_is_bogus) - die("unable to set up work tree using invalid config"); + die(_("unable to set up work tree using invalid config")); work_tree = get_git_work_tree(); git_dir = get_git_dir(); if (!is_absolute_path(git_dir)) git_dir = real_path(get_git_dir()); if (!work_tree || chdir(work_tree)) - die("This operation must be run in a work tree"); + die(_("This operation must be run in a work tree")); We have settled with lower-case letters at the beginning of error messages. (See Documentation/CodingGuidelines, "Error Messages".) You could fix that while you are touching die, die_errno, etc, messages. @@ -677,12 +677,12 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, else { char *core_worktree; if (chdir(gitdirenv)) - die_errno("Could not chdir to '%s'", gitdirenv); + die_errno(_("Cannot chdir to '%s'"), gitdirenv); I notice you change past tense to present tense in some cases. IMO, this makes the messages more consistent. Good. I'm not a friend of geeky abbreviations like "chdir" or "cwd" in user-visible messages, and I would have taken the opportunity to change the messages accordingly. This is really only my personal taste, though, and it's possible that I'm alone in this camp. if (chdir(git_work_tree_cfg)) - die_errno("Could not chdir to '%s'", git_work_tree_cfg); + die_errno(_("Cannot chdir to '%s'"), git_work_tree_cfg); core_worktree = xgetcwd(); if (chdir(cwd->buf)) - die_errno("Could not come back to cwd"); + die_errno(_("Cannot come back to cwd"); ... @@ -1207,7 +1207,7 @@ void sanitize_stdfds(void) while (fd != -1 && fd < 2) fd = dup(fd); if (fd == -1) - die_errno("open /dev/null or dup failed"); + die_errno(_("open /dev/null or dup failed")); if (fd > 2) close(fd); } @@ -1222,12 +1222,12 @@ int daemonize(void) case 0: break; case -1: - die_errno("fork failed"); + die_errno(_("fork failed")); default: exit(0); } if (setsid() == -1) - die_errno("setsid failed"); + die_errno(_("setsid failed")); Here is a certain class of errors: They should occur only rarely (actually, is that true?) Then it is useful to have the function name in the message. Which rises the question: why translate them at all? It's possible that translators turn the message into unusable gibberish just to please their language. All of this is only IMHO; I don't use translated Git. -- Hannes
Re: [PATCH 1/1] Mark messages for translations
And again, sigh: >> const char git_usage_string[] = >> const char git_more_info_string[] = > It is not obvious to me where git_usage_string is looked up in the > message catalog. It is used that way in builtin/help.c .. I wanted to be consistent with the current state of the file. This is the same way const char git_more_info_string[] is defined and initialized. Having it this way saves the lookup on each usage I guess but any performance gains will be negligible. Is there a convention? > We have settled with lower-case letters at the beginning of error > messages. (See Documentation/CodingGuidelines, "Error Messages".) You > could fix that while you are touching die, die_errno, etc, messages. Great! It will allow for further reduction in repetition of messages. > I notice you change past tense to present tense in some cases. I am reading this that messages SHOULD be in present tense (unless they MUST be in past tense). This is good. Perhaps I will look at current messages and then fix en masse (if there is sth to fix). > I'm not a friend of geeky abbreviations like "chdir" or "cwd" in > user-visible messages I agree but I would also keep in mind that using the name of the function may alllow to parametrize the messages like: Cannot execute "%s" on "%s" - for example. Anyway this is not that important for me. Will wait for other opinions. >> - die_errno("fork failed"); >> + die_errno(_("fork failed")); >> - die_errno("setsid failed"); >> + die_errno(_("setsid failed")); > it is useful to have the function name in the message. Which rises the > question:why translate them at all? Why not eat the cake while having it - one can pass function name in a message like: '"%s" failed' Regards: al_shopov
Re: [PATCH 1/1] Mark messages for translations
On Mon, Jan 15, 2018 at 12:44 AM, Alexander Shopov wrote: > Reuse already translated messages if possible > Do not translate messages aimed at developers of git A couple comments beyond those from Hannes... > Signed-off-by: Alexander Shopov > --- > diff --git a/git.c b/git.c > @@ -5,11 +5,11 @@ > const char git_usage_string[] = > + N_("git [--version] [--help] [-C ] [-c name=value]\n" Since you're touching this, perhaps take the opportunity to fix `-c name=value` to say `-c =`? > + " [--exec-path[=]] [--html-path] [--man-path] > [--info-path]\n" > + " [-p | --paginate | --no-pager] [--no-replace-objects] > [--bare]\n" > + " [--git-dir=] [--work-tree=] > [--namespace=]\n" > + "[]"); > diff --git a/setup.c b/setup.c > @@ -525,19 +525,19 @@ void read_gitfile_error_die(int error_code, const char > *path, const char *dir) > case READ_GITFILE_ERR_NOT_A_REPO: > - die("Not a git repository: %s", dir); > + die(_("Not a git repository: %s"), dir); > default: > - die("BUG: unknown error code"); > + die(_("BUG: unknown error code")); This last one is aimed at developers (indeed, "BUG" message should never been seen by end-users). I'd leave it untranslated. > } > }
Re: [PATCH 1/1] Mark messages for translations
On Mon, Jan 15, 2018 at 12:44 PM, Alexander Shopov wrote: > @@ -160,7 +160,7 @@ int check_filename(const char *prefix, const char *arg) > free(to_free); > return 0; /* file does not exist */ > } > - die_errno("failed to stat '%s'", arg); > + die_errno(_("failed to stat '%s'", arg)); > } The new ")" is at a wrong place. It should be %s'"), arg); not %s'", arg)); -- Duy
Re: [PATCH 1/1] Mark messages for translations
On Tue, Feb 6, 2018 at 1:15 AM, Alexander Shopov wrote: > Small changes in messages to fit the style and typography of rest > Reuse already translated messages if possible > Do not translate messages aimed at developers of git > Fix unit tests depending on the original string > > Signed-off-by: Alexander Shopov > --- > diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh > @@ -31,7 +31,7 @@ test_expect_success 'bad setup: invalid .git file format' ' > echo "git rev-parse accepted an invalid .git file" > false > fi && > - if ! grep "Invalid gitfile format" .err > + if ! grep "invalid gitfile format" .err Since these strings can now be translated, you'll probably need to use 'test_i18ngrep' rather than 'grep'. (See test_i18ngrep in t/test-lib.sh.) > then > echo "git rev-parse returned wrong error" > false > @@ -45,7 +45,7 @@ test_expect_success 'bad setup: invalid .git file path' ' > echo "git rev-parse accepted an invalid .git file path" > false > fi && > - if ! grep "Not a git repository" .err > + if ! grep "not a git repository" .err > then > echo "git rev-parse returned wrong error" > false > diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh > index d27f438bf..5743b482f 100755 > --- a/t/t0008-ignores.sh > +++ b/t/t0008-ignores.sh > @@ -307,7 +307,7 @@ test_expect_success_multi 'needs work tree' '' ' > cd .git && > test_check_ignore "foo" 128 > ) && > - stderr_contains "fatal: This operation must be run in a work tree" > + stderr_contains "fatal: this operation must be run in a work tree" > '
Re: [PATCH 1/1] Mark messages for translations
On Tue, Feb 06, 2018 at 02:32:42AM -0500, Eric Sunshine wrote: > > diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh > > @@ -31,7 +31,7 @@ test_expect_success 'bad setup: invalid .git file format' > > ' > > echo "git rev-parse accepted an invalid .git file" > > false > > fi && > > - if ! grep "Invalid gitfile format" .err > > + if ! grep "invalid gitfile format" .err > > Since these strings can now be translated, you'll probably need to use > 'test_i18ngrep' rather than 'grep'. (See test_i18ngrep in > t/test-lib.sh.) Good catch. Looks like there's another in t1506, which you can see with "make GETTEXT_POISON=1 test". -Peff
Re: [PATCH 1/1] Mark messages for translations
Alexander Shopov writes: > Small changes in messages to fit the style and typography of rest > Reuse already translated messages if possible > Do not translate messages aimed at developers of git > Fix unit tests depending on the original string > Use `test_i18ngrep` for tests with translatable strings > Change and verifyrest of tests via `make GETTEXT_POISON=1 test` Perhaps end each sentence with a full-stop? > diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh > index 9670e8cbe..797dcf95b 100755 > --- a/t/t0002-gitfile.sh > +++ b/t/t0002-gitfile.sh > @@ -31,7 +31,7 @@ test_expect_success 'bad setup: invalid .git file format' ' > echo "git rev-parse accepted an invalid .git file" > false > fi && > - if ! grep "Invalid gitfile format" .err > + if ! test_i18ngrep "invalid gitfile format" .err Shouldn't this rather be like so instead? if test_i18ngrep ! "invalid gitfile format" .err Ditto for the other negated use of test_i18ngrep we see in the same file in this patch.
Re: [PATCH 1/1] Mark messages for translations
Junio C Hamano writes: >> -if ! grep "Invalid gitfile format" .err >> +if ! test_i18ngrep "invalid gitfile format" .err > > Shouldn't this rather be like so instead? > > if test_i18ngrep ! "invalid gitfile format" .err > > Ditto for the other negated use of test_i18ngrep we see in the same > file in this patch. Sorry, my thinko. These two ones want to be written in the patch as-is.
Re: [PATCH 1/1] Mark messages for translations
On Fri, Feb 09, 2018 at 11:15:06AM -0800, Junio C Hamano wrote: > Junio C Hamano writes: > > >> - if ! grep "Invalid gitfile format" .err > >> + if ! test_i18ngrep "invalid gitfile format" .err > > > > Shouldn't this rather be like so instead? > > > > if test_i18ngrep ! "invalid gitfile format" .err > > > > Ditto for the other negated use of test_i18ngrep we see in the same > > file in this patch. > > Sorry, my thinko. These two ones want to be written in the patch > as-is. Yes, I think so, but we may want to avoid this anti-pattern (since usually "! test_i18ngrep" is a sign of something wrong. It seems like these tests are doing more manual reporting work than is necessary, and could just be relying on helpers to report errors. Something like the patch below, though I'm not sure if we'd want to leave it as "grep" (if applying on master), or have "test_i18ngrep" in the preimage (if basing on top of Alexander's patch). -Peff --- t/t0002-gitfile.sh | 54 +++ 1 file changed, 11 insertions(+), 43 deletions(-) diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh index 9670e8cbe6..74b7307997 100755 --- a/t/t0002-gitfile.sh +++ b/t/t0002-gitfile.sh @@ -10,15 +10,6 @@ objpath() { echo "$1" | sed -e 's|\(..\)|\1/|' } -objck() { - p=$(objpath "$1") - if test ! -f "$REAL/objects/$p" - then - echo "Object not found: $REAL/objects/$p" - false - fi -} - test_expect_success 'initial setup' ' REAL="$(pwd)/.real" && mv .git "$REAL" @@ -26,30 +17,14 @@ test_expect_success 'initial setup' ' test_expect_success 'bad setup: invalid .git file format' ' echo "gitdir $REAL" >.git && - if git rev-parse 2>.err - then - echo "git rev-parse accepted an invalid .git file" - false - fi && - if ! grep "Invalid gitfile format" .err - then - echo "git rev-parse returned wrong error" - false - fi + test_must_fail git rev-parse 2>.err && + test_i18ngrep "Invalid gitfile format" .err ' test_expect_success 'bad setup: invalid .git file path' ' echo "gitdir: $REAL.not" >.git && - if git rev-parse 2>.err - then - echo "git rev-parse accepted an invalid .git file path" - false - fi && - if ! grep "Not a git repository" .err - then - echo "git rev-parse returned wrong error" - false - fi + test_must_fail git rev-parse 2>.err && + test_i18ngrep "Not a git repository" .err ' test_expect_success 'final setup + check rev-parse --git-dir' ' @@ -60,7 +35,7 @@ test_expect_success 'final setup + check rev-parse --git-dir' ' test_expect_success 'check hash-object' ' echo "foo" >bar && SHA=$(cat bar | git hash-object -w --stdin) && - objck $SHA + test_path_is_file "$REAL/objects/$(objpath $SHA)" ' test_expect_success 'check cat-file' ' @@ -69,29 +44,22 @@ test_expect_success 'check cat-file' ' ' test_expect_success 'check update-index' ' - if test -f "$REAL/index" - then - echo "Hmm, $REAL/index exists?" - false - fi && + test_path_is_missing "$REAL/index" && rm -f "$REAL/objects/$(objpath $SHA)" && git update-index --add bar && - if ! test -f "$REAL/index" - then - echo "$REAL/index not found" - false - fi && - objck $SHA + test_path_is_file "$REAL/index" && + test_path_is_file "$REAL/objects/$(objpath $SHA)" && + false ' test_expect_success 'check write-tree' ' SHA=$(git write-tree) && - objck $SHA + test_path_is_file "$REAL/objects/$(objpath $SHA)" ' test_expect_success 'check commit-tree' ' SHA=$(echo "commit bar" | git commit-tree $SHA) && - objck $SHA + test_path_is_file "$REAL/objects/$(objpath $SHA)" ' test_expect_success 'check rev-list' '
Re: [PATCH 1/1] Mark messages for translations
Let me repeat what you said so I know how to improve the patch: @Junio: > Perhaps end each sentence with a full-stop? I should end each sentence in the *log* message with "." (rather than the translatable strings in the patch) > Shouldn't this rather be like so instead? > if test_i18ngrep ! "invalid gitfile format" .err ... > These two ones want to be written The standard negation form is: test_i18ngrep ! but I should leave the `!` in front of `test_i18ngrep` in this particular case @Jeff: > we may want to avoid this anti-pattern Current state of these tests is wrong and I should rework them. Here is what I intend to do: 1. Fix the commit message 2. Check whether I can get the tests in t0002-gitfile.sh to the standard `test_i18ngrep !` negative (i.e. without using `if`) 3. Post and ask for feedback again Kind regards: al_shopov
Re: [PATCH 1/1] Mark messages for translations
On Mon, Feb 12, 2018 at 04:03:49PM +0100, Alexander Shopov wrote: > @Jeff: > > we may want to avoid this anti-pattern > Current state of these tests is wrong and I should rework them. > > Here is what I intend to do: > 1. Fix the commit message > 2. Check whether I can get the tests in t0002-gitfile.sh to the > standard `test_i18ngrep !` negative (i.e. without using `if`) > 3. Post and ask for feedback again See the patch I posted earlier. For (2), "test_i18ngrep !" would be the wrong thing. I think you should either: - keep your patch as-is, and let Junio resolve the conflict when the two are merged - rebase your patch on top of mine. That's slightly less work for Junio, but it means that your topic cannot graduate until mine does (though hopefully mine is pretty non-controversial). I'd probably just do the first in your place, since the conflict is easy to resolve. -Peff