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

From: Emily Shaffer <emilyshaffer@google.com>
Date: Mar 24, 2021, 20:07 UTC
Message-ID: <YFucFoVsJXi1UBX+@google.com>
In-Reply-To: <CAPig+cRcWUgM=c9k-P1hGZGJEZpSgHXK=_ribJeLVsiQWV3LnQ@mail.gmail.com>

On Wed, Mar 24, 2021 at 03:23:30PM -0400, Eric Sunshine wrote:

Show 23 quoted lines

> 
> On Wed, Mar 24, 2021 at 3:12 PM Junio C Hamano <gitster@pobox.com> 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 */

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);
          }
  }