Re: [PATCH v8 04/37] hook: include hookdir hook in list

3 messages, 3 authors, 2021-03-24 · open the first message on its own page

Re: [PATCH v8 04/37] hook: include hookdir hook in list

From: Junio C Hamano <hidden>
Date: 2021-03-24 19:12:27

Emily Shaffer [off-list ref] writes:
quoted
quoted
@@ -40,10 +40,15 @@ static int list(int argc, const char **argv, const char *prefix)
 
 	list_for_each(pos, head) {
 		struct hook *item = list_entry(pos, struct hook, list);
-		if (item)
+		item = list_entry(pos, struct hook, list);
+		if (item) {
+			/* Don't translate 'hookdir' - it matches the config */
Let's prefix comments for translators with /* TRANSLATORS: .., see the
coding style doc. That's what they'll see, and this is useful to them.

Better yet have a note here about the first argument being 'system',
'local' etc., which I had to source spelunge for, and translators won't
have any idea about unless the magic parameter is documented.
It's not a comment for translators. It's a comment for someone helpful
who comes later and says "oh, none of this is marked for translation,
I'd better fix that."
Then, it is not limited to "hookdir", is it?  Resurrecting the
elided part back here:

Not just we do not want "hookdir" placed inside _(),

 			printf("%s: %s\n",
+			       (item->from_hookdir
+				? "hookdir"
+				: config_scope_name(item->origin)),
 			       item->command.buf);

we do not want the "%s: %s\n" to be placed inside _() and get munged
into "%2$s: %1$s\n" for languages that want the order swapped, for
example.

So perhaps the comment should be about the entire output, i.e.
"don't translate the output from this helper, as it is meant to be
machine parseable", or something?

Re: [PATCH v8 04/37] hook: include hookdir hook in list

From: Eric Sunshine <hidden>
Date: 2021-03-24 19:24:27

On Wed, Mar 24, 2021 at 3:12 PM Junio C Hamano [off-list ref] wrote:
Not just we do not want "hookdir" placed inside _(),

                        printf("%s: %s\n",
+                              (item->from_hookdir
+                               ? "hookdir"
+                               : config_scope_name(item->origin)),
                               item->command.buf);

we do not want the "%s: %s\n" to be placed inside _() and get munged
into "%2$s: %1$s\n" for languages that want the order swapped, for
example.

So perhaps the comment should be about the entire output, i.e.
"don't translate the output from this helper, as it is meant to be
machine parseable", or something?
Having the word "translate" in the comment automatically implies
localization, which confuses the issue. It would be clearer to avoid
that word altogether. Perhaps something along the lines of:

    /* machine-parseable output; do not apply _() localization */

Re: [PATCH v8 04/37] hook: include hookdir hook in list

From: Emily Shaffer <hidden>
Date: 2021-03-24 20:08:39

On Wed, Mar 24, 2021 at 03:23:30PM -0400, Eric Sunshine wrote:
On Wed, Mar 24, 2021 at 3:12 PM Junio C Hamano [off-list ref] wrote:
quoted
Not just we do not want "hookdir" placed inside _(),

                        printf("%s: %s\n",
+                              (item->from_hookdir
+                               ? "hookdir"
+                               : config_scope_name(item->origin)),
                               item->command.buf);

we do not want the "%s: %s\n" to be placed inside _() and get munged
into "%2$s: %1$s\n" for languages that want the order swapped, for
example.

So perhaps the comment should be about the entire output, i.e.
"don't translate the output from this helper, as it is meant to be
machine parseable", or something?
Having the word "translate" in the comment automatically implies
localization, which confuses the issue. It would be clearer to avoid
that word altogether. Perhaps something along the lines of:

    /* machine-parseable output; do not apply _() localization */
After I read Ævar's comments on the next patch in this series, I decided
to rework the comments and translation markers for this whole section.

  if (item) {
          if (item->from_hookdir) {
                  /*
                   * TRANSLATORS: do not translate 'hookdir' as
                   * it matches the config setting.
                   */
                  switch (should_run_hookdir) {
                  case HOOKDIR_NO:
                          printf(_("hookdir: %s (will not run)\n"),
                                 item->command.buf);
                          break;
                  case HOOKDIR_ERROR:
                          printf(_("hookdir: %s (will error and not run)\n"),
                                 item->command.buf);
                          break;
                  case HOOKDIR_INTERACTIVE:
                          printf(_("hookdir: %s (will prompt)\n"),
                                 item->command.buf);
                          break;
                  case HOOKDIR_WARN:
                          printf(_("hookdir: %s (will warn but run)\n"),
                                 item->command.buf);
                          break;
                  case HOOKDIR_YES:
                  /*
                   * The default behavior should agree with
                   * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just
                   * do the default behavior.
                   */
                  case HOOKDIR_UNKNOWN:
                  default:
                          printf(_("hookdir: %s\n"),
                                   item->command.buf);
                          break;
                  }
          } else {
                  /*
                   * TRANSLATORS: "<config scope>: <path>". Both fields
                   * should be left untranslated; config scope matches the
                   * output of 'git config --show-scope'. Marked for
                   * translation to provide better RTL support later.
                   */
                  printf(_("%s: %s\n"),
                          config_scope_name(item->origin),
                          item->command.buf);
          }
  }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help