(cc-ing Duy because of a mention of his nice GETTEXT_POISON tweak[*])

Jiang Xin wrote:

> Mark strings in git-rebase.sh for translation. Jonathan offers a help
> for reorgnization of the resolvemsg variable in 'git-rebase.sh', since
> there is a likely message in git-am.sh, I update it in this commit for
> consistency. And so does to 't/t0201-gettext-fallbacks.sh'.

Ah.  Looks like I tweaked the comma usage and sentence structure a
little.  Sorry, force of habit --- I shouldn't have.

> Some test scripts are affected by this update, and would fail if tested
> with GETTEXT_POISON switch turned on. Using i18n-specific test
> functions, such as test_i18ngrep, in the related test scripts will fix
> these issues.

If we're going to keep the changes together, here's how I would phrase
the commit message:

        Mark messages in git-rebase.sh for translation.  While doing this
        it was noticed that the comma usage and sentence structure of the
        resolvemsg was not quite right, so correct that and its cousins in
        git-am.sh and t/t0201-gettext-fallbacks.sh at the same time.

        Some tests would start to fail with GETTEXT_POISON turned on after
        this update.  Use test_i18ncmp and test_i18ngrep where
        appropriate to mark strings that should only be checked in the C
        locale output to avoid such issues.

> Signed-off-by: Jiang Xin <worldhello....@gmail.com>
> Reviewed-by: Stefano Lattarini <stefano.lattar...@gmail.com>
> Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

I haven't tested or reviewed this patch in detail, so even though it
looks good, I'd prefer it not to have my Reviewed-by.  (See
Documentation/SubmittingPatches: '"Reviewed-by:", unlike the other
extra tags, can only be offered by the reviewer'.)  If you'd like to
credit my help, something like "With advice from Jonathan." would be

> --- a/t/t3406-rebase-message.sh
> +++ b/t/t3406-rebase-message.sh
> @@ -64,7 +64,7 @@ test_expect_success 'rebase -n overrides config rebase.stat 
> config' '
>  test_expect_success 'rebase --onto outputs the invalid ref' '
>       test_must_fail git rebase --onto invalid-ref HEAD HEAD 2>err &&
> -     grep "invalid-ref" err
> +     test_i18ngrep "invalid-ref" err
>  '

Could we add a comment so others do not have to wonder what
human-readable message prompts the test_i18ngrep here?  e.g

        # "Does not point to a valid commit: invalid-ref"
        # NEEDSWORK: This "grep" is fine in real non-C locales, but
        # GETTEXT_POISON poisons the refname along with the enclosing
        # error message.
        test_i18ngrep invalid-ref err

In the long run we may be able to turn this back to a "grep" again,
since any reasonable translation will keep the $onto_name somewhere
in the message.  But changing it to test_i18ngrep for now is the
right thing to do, until something like Duy's more sophisticated
version of GETTEXT_POISON arrives. (<-- [*])

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to