Re: [PATCH v5 4/5] pack-bitmap.c: retrieve missing i18n translations

[Junio C Hamano <gitster@pobox.com>]

Teng Long <dyroneteng@gmail.com> writes:

> On Date: Tue, 28 Jun 2022 11:07:26 -0700, Junio C Hamano wrote:
>
>
>> The verb "retrieve" is puzzling.
>
> I use "retrieve" because I think they should be there but actually missing.
> But If it's not appropriate here I will change another word like "add".

To retrieve is to get/bring something back and regaining possession
of, which implies that the thing existed somewhere already but at a
wrong/different place, and the only thing you are doing is to move
it to the right place, but in this case, the translations did not
exist.  The patch is marking more strings for translation.  And the
act of marking them for translation will cause i18n/l10n folks to
translate these strings, which will (finally) allow _("...") to
retrieve the translated strings at runtime.

So "retrieve" is indeed involved somewhere in the process, but using
the verb skips a few steps.

    Subject: [PATCH 4/5] pack-bitmap.c: mark more strings for translations

perhaps?

>> If we were to do this, to avoid burdening translators with double
>> work, we probably would want to fix the "C" locale version of the
>> string, either as a preliminary clean-up before this step, or as
>> part of this step.  From Documentation/CodingGuidelines:
>
> Yes.
>
> Does git have any NOT "C" Locale string?

Sorry, but I am not sure what you are asking.  What I meant is that
a hunk like this from the patch in discussion:

 	if (bitmap_size < 0) {
-		error("Failed to load bitmap index (corrupted?)");
+		error(_("Failed to load bitmap index (corrupted?)"));
 		ewah_pool_free(b);
 		return NULL;
 	}

makes translators to first translate the above string, but we will
fix the "C" locale version (that is, the string inside _() that is
used as the key to the .po database when retrieving the translated
version) to follow our error message formatting convention to read
something like

	error(_("failed to load bitmap index (corrupted?)"));

or even

	error(_("failed to load bitmap index (corrupted?): '%s'"),
	      filename);

And the translators have to redo the work.  If a preliminary patch
fixed these up before bothering translators with more strings to
translate, they do not need to translate the current, known to be
faulty, version of messages.

> Another doublt is I found something like in:
>
>     File: ./contrib/completion/git-completion.bash
>     923    LANG=C LC_ALL=C git merge -s help 2>&1
>
> I think LC_ALL=C will override LANG=C in these cases, so I think
> `LC_ALL=C git merge -s help 2>&1` is OK here.

In practice, yes, but the code is following the convention to reduce
common confusion caused by leaving some lower precedence but common
environment variables (i.e. LANG) as their original values.

Does the line in the completion script have anything to do with
[PATCH 4/5], or is this merely your curiosity?  Avoid mixing in
unrelated things into the topic, which will only make the review
cycle unnecessarily longer, but raise a separate discussion if you
have to.

Thanks.