Factorization of messages with similar meaning #1088
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The patch series looks mostly good. I've left two comments on 637cc99 where you flipped the meaning in two instances. I haven't looked at why tests fail, though. |
rimrul
reviewed
Dec 2, 2021
archive.c
Outdated
| @@ -577,11 +577,11 @@ static int parse_archive_args(int argc, const char **argv, | |||
| if (remote) | |||
| die(_("Unexpected option --remote")); | |||
| if (exec) | |||
| die(_("Option --exec can only be used together with --remote")); | |||
| die(_("%s and %s are mutually exclusive"), "--exec", "--remote"); | |||
builtin/index-pack.c
Outdated
| @@ -1849,7 +1849,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) | |||
| if (from_stdin && !startup_info->have_repository) | |||
| die(_("--stdin requires a git repository")); | |||
| if (from_stdin && hash_algo) | |||
| die(_("--object-format cannot be used with --stdin")); | |||
| die(_("%s requires %s"), "--object-format", "--stdin"); | |||
There was a problem hiding this comment.
Should be "%s and %s are mutually exclusive".
|
Thank you for catching these mistakes. The build is failing because some error messages are checked. |
jnavila
force-pushed
the
i18n-refactor
branch
from
cee3ad5
to
7d97cbe
Compare
|
/submit |
|
Submitted as pull.1088.git.1638514909.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Jeff King wrote (reply to this): |
|
User |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): |
|
User |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
On the Git mailing list, Jean-Noël AVILA wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): |
|
This branch is now known as |
|
This patch series was integrated into seen via git@e5a04d9. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
This patch series was integrated into seen via git@336152c. |
|
This patch series was integrated into seen via git@a61e664. |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
builtin/checkout.c
Outdated
| @@ -1621,7 +1621,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, | |||
| cb_option, toupper(cb_option)); | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Fri, Dec 03 2021, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> Use static strings for constant parts of the sentences.
>
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
> builtin/checkout.c | 2 +-
> builtin/diff-tree.c | 2 +-
> builtin/fetch.c | 2 +-
> builtin/init-db.c | 2 +-
> builtin/log.c | 4 ++--
> builtin/submodule--helper.c | 4 ++--
> builtin/worktree.c | 2 +-
> range-diff.c | 2 +-
> 8 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index cbf73b8c9f6..4bd8a57f190 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1621,7 +1621,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
> cb_option, toupper(cb_option));
>
> if (opts->overlay_mode == 1 && opts->patch_mode)
> - die(_("-p and --overlay are mutually exclusive"));
> + die(_("%s and %s are mutually exclusive"), "-p", "--overlay");
It's good to do all of these, but I think we should really quote the
'%s' while at it. It also helps translators, who without that won't know
(without jumping to the source) if %s and %s are "walking" and "chewing
gum" or something like a CLI option that's quoted :)
There was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Tuesday, 7 December 2021 19:07:41 CET �var Arnfj�r� Bjarmason wrote:
>
> On Fri, Dec 03 2021, Jean-No�l Avila via GitGitGadget wrote:
>
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
> >
> > Use static strings for constant parts of the sentences.
> >
> > Signed-off-by: Jean-No�l Avila <jn.avila@free.fr>
> > ---
> > builtin/checkout.c | 2 +-
> > builtin/diff-tree.c | 2 +-
> > builtin/fetch.c | 2 +-
> > builtin/init-db.c | 2 +-
> > builtin/log.c | 4 ++--
> > builtin/submodule--helper.c | 4 ++--
> > builtin/worktree.c | 2 +-
> > range-diff.c | 2 +-
> > 8 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/builtin/checkout.c b/builtin/checkout.c
> > index cbf73b8c9f6..4bd8a57f190 100644
> > --- a/builtin/checkout.c
> > +++ b/builtin/checkout.c
> > @@ -1621,7 +1621,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
> > cb_option, toupper(cb_option));
> >
> > if (opts->overlay_mode == 1 && opts->patch_mode)
> > - die(_("-p and --overlay are mutually exclusive"));
> > + die(_("%s and %s are mutually exclusive"), "-p", "--overlay");
>
> It's good to do all of these, but I think we should really quote the
> '%s' while at it. It also helps translators, who without that won't know
> (without jumping to the source) if %s and %s are "walking" and "chewing
> gum" or something like a CLI option that's quoted :)
>
With other comments included, the proposition for this factorization would be:
"the options '%s' and '%s' cannot be used together"
|
User |
| @@ -185,7 +185,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base, | |||
|
|
|||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Fri, Dec 03 2021, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
> archive.c | 4 ++--
> builtin/fetch.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/archive.c b/archive.c
> index 10376be7161..f1208beacff 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -185,7 +185,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
>
> buffer = object_file_to_archive(args, path.buf, oid, mode, &type, &size);
> if (!buffer)
> - return error(_("cannot read %s"), oid_to_hex(oid));
> + return error(_("cannot read '%s'"), oid_to_hex(oid));
Re my just-sent comment, i.e. quote things like that, but between this
and the first patch there's a bunch that don't use quotes that could use
them...
| @@ -522,7 +522,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) | |||
| finalize_colopts(&colopts, -1); | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Fri, Dec 03 2021, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
> builtin/tag.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 41941d5129f..6415d6c81a2 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -543,13 +543,13 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
> goto cleanup;
> }
> if (filter.lines != -1)
> - die(_("-n option is only allowed in list mode"));
> + die(_("%s option is only allowed in list mode"), "-n");
> if (filter.with_commit)
> - die(_("--contains option is only allowed in list mode"));
> + die(_("%s option is only allowed in list mode"), "--contains");
> if (filter.no_commit)
> - die(_("--no-contains option is only allowed in list mode"));
> + die(_("%s option is only allowed in list mode"), "--no-contains");
> if (filter.points_at.nr)
> - die(_("--points-at option is only allowed in list mode"));
> + die(_("%s option is only allowed in list mode"), "--points-at");
> if (filter.reachable_from || filter.unreachable_from)
> die(_("--merged and --no-merged options are only allowed in list mode"));
> if (cmdmode == 'd') {
Since for all of these we're asking translators to re-do some work
(albeit with translation memory) this could use a bit of grammar
improvement. E.g.:
_("the '%s' option is only allowed in list mode'")
I.e. "blah option is only" without a "the" is a bit odd.
But also for this & various other boilerplate in this series, I think it
would be much better as say:
const char *only_in_list = NULL;
if (...)
only_in_list = "-n";
else if (...)
only_in_list = "--contains";
[...]
if (only_in_list)
die(__("the '%s' option [...]"), only_in_list);
I.e. we're buying ourselves the chance to easily get a rid of a lot of
this repetition, but aren't using it. I think a series like this should
probably resist some big refactorings, but things like that would be
pretty easy & make the code more readable.
| @@ -341,7 +341,7 @@ static int objectsize_atom_parser(struct ref_format *format, struct used_atom *a | |||
| else | |||
There was a problem hiding this comment.
On the Git mailing list, Ævar Arnfjörð Bjarmason wrote (reply to this):
On Fri, Dec 03 2021, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <jn.avila@free.fr>
>
> Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
> ---
> ref-filter.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/ref-filter.c b/ref-filter.c
> index 08a3f839c97..554c2ba1b17 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -341,7 +341,7 @@ static int objectsize_atom_parser(struct ref_format *format, struct used_atom *a
> else
> oi.info.disk_sizep = &oi.disk_size;
> } else
> - return strbuf_addf_ret(err, -1, _("unrecognized %%(objectsize) argument: %s"), arg);
> + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), "objectsize", arg);
> return 0;
> }
>
> @@ -374,7 +374,7 @@ static int subject_atom_parser(struct ref_format *format, struct used_atom *atom
> else if (!strcmp(arg, "sanitize"))
> atom->u.contents.option = C_SUB_SANITIZE;
> else
> - return strbuf_addf_ret(err, -1, _("unrecognized %%(subject) argument: %s"), arg);
> + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), "subject", arg);
> return 0;
> }
>
> @@ -428,7 +428,7 @@ static int contents_atom_parser(struct ref_format *format, struct used_atom *ato
> if (strtoul_ui(arg, 10, &atom->u.contents.nlines))
> return strbuf_addf_ret(err, -1, _("positive value expected contents:lines=%s"), arg);
> } else
> - return strbuf_addf_ret(err, -1, _("unrecognized %%(contents) argument: %s"), arg);
> + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), "contents", arg);
> return 0;
> }
>
> @@ -440,7 +440,7 @@ static int raw_atom_parser(struct ref_format *format, struct used_atom *atom,
> else if (!strcmp(arg, "size"))
> atom->u.raw_data.option = RAW_LENGTH;
> else
> - return strbuf_addf_ret(err, -1, _("unrecognized %%(raw) argument: %s"), arg);
> + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), "raw", arg);
> return 0;
> }
>
> @@ -531,7 +531,7 @@ static int align_atom_parser(struct ref_format *format, struct used_atom *atom,
> else if ((position = parse_align_position(s)) >= 0)
> align->position = position;
> else {
> - strbuf_addf(err, _("unrecognized %%(align) argument: %s"), s);
> + strbuf_addf(err, _("unrecognized %%(%s) argument: %s"), "align", s);
> string_list_clear(¶ms, 0);
> return -1;
> }
> @@ -557,7 +557,7 @@ static int if_atom_parser(struct ref_format *format, struct used_atom *atom,
> } else if (skip_prefix(arg, "notequals=", &atom->u.if_then_else.str)) {
> atom->u.if_then_else.cmp_status = COMPARE_UNEQUAL;
> } else
> - return strbuf_addf_ret(err, -1, _("unrecognized %%(if) argument: %s"), arg);
> + return strbuf_addf_ret(err, -1, _("unrecognized %%(%s) argument: %s"), "if", arg);
> return 0;
> }
Maybe too big a refactoring, but doesn't the parent function here know
the "name" and could just pass it along?
Or actually not pass it at all and just have all of these be say:
return -2;
And we'd do the error one caller above this if we get a return code of
-2.
|
This patch series was integrated into seen via git@190b117. |
|
This patch series was integrated into seen via git@f1e1620. |
|
There was a status update in the "New Topics" section about the branch Similar message templates have been consolidated so that translators need to work on fewer number of messages. Needs review. source: <pull.1088.git.1638514909.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@f8dc6ad. |
|
This patch series was integrated into seen via git@5e0f758. |
|
This patch series was integrated into seen via git@75209e8. |
|
There was a status update in the "Cooking" section about the branch Similar message templates have been consolidated so that translators need to work on fewer number of messages. cf. <48262743.WQR3eRXon5@cayenne> source: <pull.1088.v4.git.1641143745.gitgitgadget@gmail.com> |
Use placeholders for constant tokens. The strings are turned into "cannot be used together" Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
They are all replaced by "the option '%s' requires '%s'", which is a new string but replaces 17 previous unique strings. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
Even if some of these messages are not subject to gettext i18n, this helps bring a single style of message for a given error type. Signed-off-by: Jean-Noël Avila <jn.avila@free.fr>
|
This patch series was integrated into seen via git@945f0e3. |
|
/submit |
|
Submitted as pull.1088.v5.git.1641412944.gitgitgadget@gmail.com To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Johannes Sixt wrote (reply to this): |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): |
|
This patch series was integrated into seen via git@94e0d6b. |
|
This patch series was integrated into seen via git@1bcea0d. |
|
There was a status update in the "Cooking" section about the branch Similar message templates have been consolidated so that translators need to work on fewer number of messages. Will merge to 'next'. source: <pull.1088.v5.git.1641412944.gitgitgadget@gmail.com> |
|
This patch series was integrated into seen via git@f7ea9db. |
|
This patch series was integrated into next via git@6440d8f. |
|
This patch series was integrated into seen via git@c17de5a. |
|
This patch series was integrated into next via git@c17de5a. |
|
This patch series was integrated into master via git@c17de5a. |
|
Closed via c17de5a. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This series is a meager attempt at rationalizing a small fraction of the internationalized messages. Sorry in advance for the dull task of reviewing these insipide patches.
Doing so has some positive effects:
Changes since V1:
Changes since V2:
Changes since V3:
Changes since V4:
cc: Jeff King peff@peff.net
cc: Johannes Sixt j6t@kdbg.org
cc: Ævar Arnfjörð Bjarmason avarab@gmail.com
cc: René Scharfe l.s.r@web.de