Teng Long <dyroneteng@xxxxxxxxx> 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.