mbox series

[v5,00/11] Factorization of messages with similar meaning

Message ID pull.1088.v5.git.1641412944.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series Factorization of messages with similar meaning | expand

Message

Philippe Blain via GitGitGadget Jan. 5, 2022, 8:02 p.m. UTC
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:

 * non-translatable constant strings are kept out of the way for translators
 * messages with identical meaning are built identically
 * the total number of messages to translate is decreased.

Changes since V1:

 * took into account the comments, except for ref-filter.c where the
   proposed refactoring is not obvious.
 * added even more strings to the "cannot be used together" crowd.

Changes since V2:

 * fixed change of behaviour in tag.c
 * reverted sam changes as per Johannes Sixt comments

Changes since V3:

 * apply Oxford comma where needed
 * switch all options to " '%s' " style where i18n is applied.

Changes since V4:

 * Apply changes by René on tag.c
 * cosmetic changes

Jean-Noël Avila (11):
  i18n: refactor "foo and bar are mutually exclusive"
  i18n: refactor "%s, %s and %s are mutually exclusive"
  i18n: turn "options are incompatible" into "cannot be used together"
  i18n: standardize "cannot open" and "cannot read"
  i18n: tag.c factorize i18n strings
  i18n: factorize "--foo requires --bar" and the like
  i18n: factorize "no directory given for --foo"
  i18n: refactor "unrecognized %(foo) argument" strings
  i18n: factorize "--foo outside a repository"
  i18n: ref-filter: factorize "%(foo) atom used without %(bar) atom"
  i18n: turn even more messages into "cannot be used together" ones

 apply.c                                   |  8 +++----
 archive.c                                 |  8 +++----
 builtin/add.c                             | 14 ++++++------
 builtin/am.c                              |  6 ++---
 builtin/branch.c                          |  2 +-
 builtin/cat-file.c                        |  2 +-
 builtin/checkout.c                        | 18 +++++++--------
 builtin/clone.c                           |  6 ++---
 builtin/commit.c                          | 17 +++++++-------
 builtin/describe.c                        |  6 ++---
 builtin/diff-tree.c                       |  2 +-
 builtin/difftool.c                        |  4 ++--
 builtin/fast-export.c                     |  4 ++--
 builtin/fetch.c                           |  8 +++----
 builtin/index-pack.c                      |  4 ++--
 builtin/init-db.c                         |  2 +-
 builtin/log.c                             |  8 +++----
 builtin/ls-files.c                        |  2 +-
 builtin/merge.c                           |  4 ++--
 builtin/pack-objects.c                    |  2 +-
 builtin/push.c                            |  8 +++----
 builtin/rebase.c                          | 10 ++++-----
 builtin/repack.c                          |  4 ++--
 builtin/reset.c                           | 10 ++++-----
 builtin/rev-list.c                        |  4 ++--
 builtin/rm.c                              |  4 ++--
 builtin/show-branch.c                     |  4 ++--
 builtin/stash.c                           |  8 +++----
 builtin/submodule--helper.c               |  4 ++--
 builtin/tag.c                             | 27 ++++++++++++++---------
 builtin/worktree.c                        |  6 ++---
 diff.c                                    | 12 ++++++----
 fetch-pack.c                              |  2 +-
 git.c                                     |  6 ++---
 http-fetch.c                              |  4 ++--
 range-diff.c                              |  2 +-
 ref-filter.c                              | 22 +++++++++---------
 revision.c                                | 22 +++++++++---------
 t/t0001-init.sh                           |  2 +-
 t/t2025-checkout-no-overlay.sh            |  2 +-
 t/t2026-checkout-pathspec-file.sh         |  8 +++----
 t/t2072-restore-pathspec-file.sh          |  6 ++---
 t/t3431-rebase-fork-point.sh              |  2 +-
 t/t3601-rm-pathspec-file.sh               |  4 ++--
 t/t3704-add-pathspec-file.sh              | 10 ++++-----
 t/t3909-stash-pathspec-file.sh            |  6 ++---
 t/t4209-log-pickaxe.sh                    | 10 ++++-----
 t/t5606-clone-options.sh                  |  4 ++--
 t/t7107-reset-pathspec-file.sh            |  6 ++---
 t/t7500-commit-template-squash-signoff.sh | 11 ++++-----
 t/t7526-commit-pathspec-file.sh           | 10 ++++-----
 51 files changed, 189 insertions(+), 178 deletions(-)


base-commit: 2ae0a9cb8298185a94e5998086f380a355dd8907
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1088%2Fjnavila%2Fi18n-refactor-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1088/jnavila/i18n-refactor-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1088

Range-diff vs v4:

  1:  05af90f5814 =  1:  05af90f5814 i18n: refactor "foo and bar are mutually exclusive"
  2:  e307ea9b998 !  2:  f6a5332d310 i18n: refactor "%s, %s and %s are mutually exclusive"
     @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
       	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
       	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
      -		die(_("-b, -B, and --detach are mutually exclusive"));
     -+		die(_("options '%s', '%s', and '%s' cannot be used together"),"-b", "-B", "--detach");
     ++		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
       	if (lock_reason && !keep_locked)
       		die(_("--reason requires --lock"));
       	if (lock_reason)
  3:  d5bfa26e992 !  3:  80390ce08ed i18n: turn "options are incompatible" into "cannot be used together"
     @@ apply.c: int check_apply_state(struct apply_state *state, int force_apply)
       
       	if (state->apply_with_reject && state->threeway)
      -		return error(_("--reject and --3way cannot be used together."));
     -+		return error(_("options '%s' and '%s' cannot be used together"), "--reject",  "--3way");
     ++		return error(_("options '%s' and '%s' cannot be used together"), "--reject", "--3way");
       	if (state->threeway) {
       		if (is_not_gitdir)
       			return error(_("--3way outside a repository"));
  4:  51c53f01dd8 =  4:  49e41dc136f i18n: standardize "cannot open" and "cannot read"
  5:  a9d8a50d666 !  5:  ad58bc8d8a9 i18n: tag.c factorize i18n strings
     @@ builtin/tag.c: int cmd_tag(int argc, const char **argv, const char *prefix)
      -		die(_("--no-contains option is only allowed in list mode"));
      -	if (filter.points_at.nr)
      -		die(_("--points-at option is only allowed in list mode"));
     +-	if (filter.reachable_from || filter.unreachable_from)
     +-		die(_("--merged and --no-merged options are only allowed in list mode"));
      +		only_in_list = "-n";
      +	else if (filter.with_commit)
      +		only_in_list = "--contains";
     @@ builtin/tag.c: int cmd_tag(int argc, const char **argv, const char *prefix)
      +		only_in_list = "--no-contains";
      +	else if (filter.points_at.nr)
      +		only_in_list = "--points-at";
     ++	else if (filter.reachable_from)
     ++		only_in_list = "--merged";
     ++	else if  (filter.unreachable_from)
     ++		only_in_list = "--no-merged";
      +	if (only_in_list)
      +		die(_("the '%s' option is only allowed in list mode"), only_in_list);
     - 	if (filter.reachable_from || filter.unreachable_from)
     --		die(_("--merged and --no-merged options are only allowed in list mode"));
     -+		die(_("'--merged' and '--no-merged' options are only allowed in list mode"));
       	if (cmdmode == 'd') {
       		ret = delete_tags(argv);
       		goto cleanup;
  6:  969946274aa !  6:  fd27beb3f2b i18n: factorize "--foo requires --bar" and the like
     @@ builtin/stash.c: static int push_stash(int argc, const char **argv, const char *
       ## builtin/worktree.c ##
      @@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix)
       	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
     - 		die(_("options '%s', '%s', and '%s' cannot be used together"),"-b", "-B", "--detach");
     + 		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
       	if (lock_reason && !keep_locked)
      -		die(_("--reason requires --lock"));
      +		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
  7:  052dc06beeb =  7:  08f5471aeaa i18n: factorize "no directory given for --foo"
  8:  59e1e8aa1b4 =  8:  437aadbc2c9 i18n: refactor "unrecognized %(foo) argument" strings
  9:  39e375c68ab !  9:  7d97ee5dd18 i18n: factorize "--foo outside a repository"
     @@ Commit message
      
       ## apply.c ##
      @@ apply.c: int check_apply_state(struct apply_state *state, int force_apply)
     - 		return error(_("options '%s' and '%s' cannot be used together"), "--reject",  "--3way");
     + 		return error(_("options '%s' and '%s' cannot be used together"), "--reject", "--3way");
       	if (state->threeway) {
       		if (is_not_gitdir)
      -			return error(_("--3way outside a repository"));
 10:  c6161bc3f1a = 10:  226c105559c i18n: ref-filter: factorize "%(foo) atom used without %(bar) atom"
 11:  4fab0db3cc4 = 11:  8bcc814ba00 i18n: turn even more messages into "cannot be used together" ones

Comments

Johannes Sixt Jan. 5, 2022, 8:58 p.m. UTC | #1
Am 05.01.22 um 21:02 schrieb Jean-Noël Avila via GitGitGadget:
> 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:
> 
>  * non-translatable constant strings are kept out of the way for translators
>  * messages with identical meaning are built identically
>  * the total number of messages to translate is decreased.
> 
> Changes since V1:
> 
>  * took into account the comments, except for ref-filter.c where the
>    proposed refactoring is not obvious.
>  * added even more strings to the "cannot be used together" crowd.
> 
> Changes since V2:
> 
>  * fixed change of behaviour in tag.c
>  * reverted sam changes as per Johannes Sixt comments
> 
> Changes since V3:
> 
>  * apply Oxford comma where needed
>  * switch all options to " '%s' " style where i18n is applied.
> 
> Changes since V4:
> 
>  * Apply changes by René on tag.c
>  * cosmetic changes

This round looks good to me, with one caveat: I am not a translator, nor
do I use a translated version of Git. So, I haven't verified the claim
that the number translatable strings was reduced greatly, nor whether
there are any accidential duplicates due to typos. I infer the
correctness only by looking at the changes.

There's one small nit visible in the range-diff; not worth a reroll IMHO:

>   5:  a9d8a50d666 !  5:  ad58bc8d8a9 i18n: tag.c factorize i18n strings
>      @@ builtin/tag.c: int cmd_tag(int argc, const char **argv, const char *prefix)
>       -		die(_("--no-contains option is only allowed in list mode"));
>       -	if (filter.points_at.nr)
>       -		die(_("--points-at option is only allowed in list mode"));
>      +-	if (filter.reachable_from || filter.unreachable_from)
>      +-		die(_("--merged and --no-merged options are only allowed in list mode"));
>       +		only_in_list = "-n";
>       +	else if (filter.with_commit)
>       +		only_in_list = "--contains";
>      @@ builtin/tag.c: int cmd_tag(int argc, const char **argv, const char *prefix)
>       +		only_in_list = "--no-contains";
>       +	else if (filter.points_at.nr)
>       +		only_in_list = "--points-at";
>      ++	else if (filter.reachable_from)
>      ++		only_in_list = "--merged";
>      ++	else if  (filter.unreachable_from)

An extra blank after the 'if'.

>      ++		only_in_list = "--no-merged";
>       +	if (only_in_list)
>       +		die(_("the '%s' option is only allowed in list mode"), only_in_list);
>      - 	if (filter.reachable_from || filter.unreachable_from)
>      --		die(_("--merged and --no-merged options are only allowed in list mode"));
>      -+		die(_("'--merged' and '--no-merged' options are only allowed in list mode"));
>        	if (cmdmode == 'd') {
>        		ret = delete_tags(argv);
>        		goto cleanup;

-- Hannes
Junio C Hamano Jan. 5, 2022, 9:31 p.m. UTC | #2
Johannes Sixt <j6t@kdbg.org> writes:

>>  * Apply changes by René on tag.c
>>  * cosmetic changes
>
> This round looks good to me, with one caveat: I am not a translator, nor
> do I use a translated version of Git. So, I haven't verified the claim
> that the number translatable strings was reduced greatly, nor whether
> there are any accidential duplicates due to typos. I infer the
> correctness only by looking at the changes.

Thanks, both.  Will queue with a touch-up for that doubled SP.