git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] builtin/diagnose.c: don't translate the two mode values
@ 2022-09-20  5:06 Alex Henrie
  2022-09-20 12:35 ` Derrick Stolee
  0 siblings, 1 reply; 3+ messages in thread
From: Alex Henrie @ 2022-09-20  5:06 UTC (permalink / raw)
  To: git, vdye, derrickstolee, gitster; +Cc: Alex Henrie

These strings are not translatable in the diagnose_options array in
diagnose.c. Don't translate them in builtin/diagnose.c either.

Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
---
 builtin/diagnose.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/diagnose.c b/builtin/diagnose.c
index cd260c2015..576e0e8e38 100644
--- a/builtin/diagnose.c
+++ b/builtin/diagnose.c
@@ -22,7 +22,7 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
 			   N_("specify a destination for the diagnostics archive")),
 		OPT_STRING('s', "suffix", &option_suffix, N_("format"),
 			   N_("specify a strftime format suffix for the filename")),
-		OPT_CALLBACK_F(0, "mode", &mode, N_("(stats|all)"),
+		OPT_CALLBACK_F(0, "mode", &mode, "(stats|all)",
 			       N_("specify the content of the diagnostic archive"),
 			       PARSE_OPT_NONEG, option_parse_diagnose),
 		OPT_END()
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] builtin/diagnose.c: don't translate the two mode values
  2022-09-20  5:06 [PATCH] builtin/diagnose.c: don't translate the two mode values Alex Henrie
@ 2022-09-20 12:35 ` Derrick Stolee
  2022-09-20 17:54   ` Alex Henrie
  0 siblings, 1 reply; 3+ messages in thread
From: Derrick Stolee @ 2022-09-20 12:35 UTC (permalink / raw)
  To: Alex Henrie, git, vdye, gitster

On 9/20/2022 1:06 AM, Alex Henrie wrote:
> These strings are not translatable in the diagnose_options array in
> diagnose.c. Don't translate them in builtin/diagnose.c either.
> 
> Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
> ---
>  builtin/diagnose.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/diagnose.c b/builtin/diagnose.c
> index cd260c2015..576e0e8e38 100644
> --- a/builtin/diagnose.c
> +++ b/builtin/diagnose.c
> @@ -22,7 +22,7 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
>  			   N_("specify a destination for the diagnostics archive")),
>  		OPT_STRING('s', "suffix", &option_suffix, N_("format"),
>  			   N_("specify a strftime format suffix for the filename")),
> -		OPT_CALLBACK_F(0, "mode", &mode, N_("(stats|all)"),
> +		OPT_CALLBACK_F(0, "mode", &mode, "(stats|all)",
>  			       N_("specify the content of the diagnostic archive"),

In terms of the existing string, this makes sense. Initially, I
thought that maybe we would rather have N_("mode") to match the
N_("format") in the above. Then, I went poking around to see what
patterns we had for these across the codebase (never a good idea).

An example similar to what you propose exists in builtin/branch.c:

		OPT_CALLBACK_F('t', "track",  &track, "(direct|inherit)",
			N_("set branch tracking configuration"),
			PARSE_OPT_OPTARG,
			parse_opt_tracking_mode),

or this instance in builtin/checkout-index.c:

		OPT_CALLBACK_F(0, "stage", NULL, "(1|2|3|all)",
			N_("copy out the files from named stage"),

In diff.c, the descriptors exist in angle brackets, so the right thing
would be N_("<mode>"). This seems non-standard compared to most other
places.

Here is a similar stale translatable regex in diff.c:

		OPT_CALLBACK_F(0, "diff-filter", options, N_("[(A|C|D|M|R|T|U|X|B)...[*]]"),
			       N_("select files by diff type"),

So if you are looking into these kinds of replacements, it might be
good to add instances like this. They are less important to the 2.38.0
release, though.

This long-winded email is all just to say that I've looked into the
standard way to handle this and agree that you are changing the code
to match our best practices.

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] builtin/diagnose.c: don't translate the two mode values
  2022-09-20 12:35 ` Derrick Stolee
@ 2022-09-20 17:54   ` Alex Henrie
  0 siblings, 0 replies; 3+ messages in thread
From: Alex Henrie @ 2022-09-20 17:54 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git, vdye, gitster

On Tue, Sep 20, 2022 at 6:35 AM Derrick Stolee <derrickstolee@github.com> wrote:
>
> In diff.c, the descriptors exist in angle brackets, so the right thing
> would be N_("<mode>"). This seems non-standard compared to most other
> places.

I don't know which form is preferred in the code, but I did find that
usage_argh adds the angle brackets if they are not already present:

static int usage_argh(const struct option *opts, FILE *outfile)
{
    const char *s;
    int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
        !opts->argh || !!strpbrk(opts->argh, "()<>[]|");
    if (opts->flags & PARSE_OPT_OPTARG)
        if (opts->long_name)
            s = literal ? "[=%s]" : "[=<%s>]";
        else
            s = literal ? "[%s]" : "[<%s>]";
    else
        s = literal ? " %s" : " <%s>";
    return utf8_fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
}

> Here is a similar stale translatable regex in diff.c:
>
>                 OPT_CALLBACK_F(0, "diff-filter", options, N_("[(A|C|D|M|R|T|U|X|B)...[*]]"),
>                                N_("select files by diff type"),
>
> So if you are looking into these kinds of replacements, it might be
> good to add instances like this. They are less important to the 2.38.0
> release, though.

Yeah, that one probably shouldn't be translatable either.

> This long-winded email is all just to say that I've looked into the
> standard way to handle this and agree that you are changing the code
> to match our best practices.

Thanks! I appreciate the feedback.

-Alex

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-09-20 17:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20  5:06 [PATCH] builtin/diagnose.c: don't translate the two mode values Alex Henrie
2022-09-20 12:35 ` Derrick Stolee
2022-09-20 17:54   ` Alex Henrie

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).