Re: [PATCH v2 1/2] object.[ch]: mark object type names for translation

From: Jeff King <peff@peff.net>
Date: Oct 6, 2021, 19:05 UTC
Message-ID: <YV3zZFOJd6blVGXn@coredump.intra.peff.net>
In-Reply-To: <patch-v2-1.2-55bde16aa23-20211004T142523Z-avarab@gmail.com>

On Mon, Oct 04, 2021 at 04:27:01PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 6 quoted lines

> Mark the "commit", "tree", "blob" and "tag" types for translation, and
> add an extern "unknown type" string for the OBJ_NONE case.
> 
> It is usually bad practice to translate individual words like this,
> but for e.g. the list list output emitted by the "short object ID dead
> is ambiguous" advice it makes sense.

We already seem to have a translatable string for "commit", but if I look at say es.po, the translation is "confirmar", which is considering it a verb. Now my Spanish is pretty rusty, so it's possible this works as a noun, too. But if I look at other messages, like:

  #: builtin/commit.c:1623
  msgid "override date for commit"
  msgstr "sobrescribe la fecha del commit"

  #: builtin/commit.c:1626
  msgid "reuse message from specified commit"
  msgstr "reusar el mensaje de un commit específico"

then it's clear that "commit" as a noun is translated as "commit". I'm not sure what facilities (if any) there are in gettext for having the same string in different contexts.

I do note that this is already a problem. Of the five spots listed:

  #: builtin/commit.c:1625 builtin/commit.c:1626 builtin/commit.c:1632
  #: parse-options.h:329 ref-filter.h:90
  msgid "commit"
  msgstr "confirmar"

They all appear to want is as a noun. So maybe this is just mis-translated for Spanish. It does feel like an accident in the making, though.

Show 6 quoted lines

> A subsequent commit will make that output translatable, and use these
> translation markings to do so. Well, we won't use "commit", but let's
> mark it up anyway for consistency. It'll probably come in handy sooner
> than later to have it already be translated, and it's to much of a
> burden to place on translators if they're translating the other three
> object types anyway.

I do wonder how useful it is to translate these type names in general. Especially as used in this series, they're really technical terms, and you are not going to escape the name "git commit" as a command. But I don't ever use translated Git, so I'm not sure my opinion is all that meaningful there.

Show 5 quoted lines

> Aside: I think it would probably make sense to change the "NULL" entry
> for type_name() to be the "unknown type". I've ran into cases where
> type_name() was unconditionally interpolated in e.g. an sprintf()
> format, but let's leave that for #leftoverbits as that would be
> changing the behavior of the type_name() function.

IMHO this would be a bad idea. Even if there is a spot that uses the result without checking for NULL, I'd much rather have Git segfault than say, write out an object with a bogus name (as it would in index_mem(), for example). So you really have to look over every caller, at which point you may as well adjust the ones that aren't checking for NULL.

Now if you introduced type_name_human(), which auto-translated and converted NULL to "unknown", then that would be easy to plug in appropriately as you audited the callers.

Show 20 quoted lines

>  static const char *object_type_strings[] = {
>  	NULL,		/* OBJ_NONE = 0 */
> -	"commit",	/* OBJ_COMMIT = 1 */
> -	"tree",		/* OBJ_TREE = 2 */
> -	"blob",		/* OBJ_BLOB = 3 */
> -	"tag",		/* OBJ_TAG = 4 */
> +	/*
> +	 * TRANSLATORS: "commit", "tree", "blob" and "tag" are the
> +	 * name of Git's object types. These names are interpolated
> +	 * stand-alone when doing so is unambiguous for translation
> +	 * and doesn't require extra context. E.g. as part of an
> +	 * already-translated string that needs to have a type name
> +	 * quoted verbatim, or the short description of a command-line
> +	 * option expecting a given type.
> +	 */
> +	N_("commit"),	/* OBJ_COMMIT = 1 */
> +	N_("tree"),	/* OBJ_TREE = 2 */
> +	N_("blob"),	/* OBJ_BLOB = 3 */
> +	N_("tag"),	/* OBJ_TAG = 4 */
>  };

This does make me feel slightly uneasy, just because so many parts of Git rely on these _not_ being translated. But I see in your other response that N_() really does nothing. So aside from possibly misleading readers of the code, I think this is probably OK.

-Peff