Remove names from translatable strings by hebasto · Pull Request #20404 · bitcoin/bitcoin

hebasto · 2020-11-16T18:59:31Z

This PR removes the following tokens from translatable strings:

command line option names
RPC names
debug log file name

All of them are untranslatable by their nature.

The Translation Strings Policy updated accordingly.

This was done while reviewing #20401. Good to get it just after 0.21 branch off :)

Command line option name must not be a part of the translatable string, as it is untranslatable by its nature.

RPC name must not be a part of the translatable string, as it is untranslatable by its nature.

Added requirement to do not translate names.

DrahtBot · 2020-11-17T01:17:21Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

refactor: split CWallet::Create #20773 (refactor: split CWallet::Create by S3RK)
wallet, refactor: return out-params of CreateTransaction() as optional struct #20640 (wallet, refactor: return out-params of CreateTransaction() as optional struct by theStack)
wallet: check for non-representable CFeeRates #20546 (wallet: check for non-representable CFeeRates by jonatack)
Per-Peer Message Capture #19509 (Per-Peer Message Capture by troygiorshev)
torcontrol : avoid to set wrong outbound proxy and network settings when creating an inbound onion service. #19358 (torcontrol : avoid to set wrong outbound proxy and network settings when creating an inbound onion service. by Saibato)
Add Single Random Draw as an additional coin selection algorithm #17526 (Use Single Random Draw In addition to knapsack as coin selection fallback by achow101)
Use effective values throughout coin selection #17331 (Use effective values throughout coin selection by achow101)
Allow maintaining the blockfilterindex when using prune #15946 (Allow maintaining the blockfilterindex when using prune by jonasschnelli)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

decryp2kanon · 2020-11-18T08:13:32Z

ACK c20e3db

Good idea 👍

decryp2kanon · 2020-11-18T08:14:26Z

src/wallet/wallet.cpp


            if (rescan_height != block_height) {
-                error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)");
+                error = strprintf(_("Prune: last wallet synchronisation goes beyond pruned data. You need to %s (download the whole block chain again in case of pruned node)."), "-reindex");


nit: blockchain vs block chain

https://github.com/bitcoinops/bitcoinops.github.io/blob/master/STYLE.md

This isn't optech

maflcko · 2020-11-18T08:27:53Z

If we want to avoid this in the future, would a linter be appropriate?

laanwj · 2020-11-18T11:25:42Z

I understand the idea but… Doesn't this make it much harder for translators to understand what is being translated? A lot of context information goes missing by replacing these with placeholders.
(the most common kind of question that I get on transifex is already "I don't understand the context of this message, please explain". It might also result in less specific and relevant translations.)

luke-jr

Concept ACK

Doesn't this make it much harder for translators to understand what is being translated? A lot of context information goes missing by replacing these with placeholders.

Qt lets you specify an additional string seen only by translators for context. Does Transifex support that? Can we add it to our utils too?

luke-jr · 2020-11-24T23:39:01Z

src/init.cpp

    if (strSubVersion.size() > MAX_SUBVERSION_LENGTH) {
-        return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of uacomments."),
-            strSubVersion.size(), MAX_SUBVERSION_LENGTH));
+        return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of %s."),


Suggested change

return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of %s."),

return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of %s options."),

luke-jr · 2020-11-24T23:39:17Z

doc/translation_strings_policy.md

+
+The following tokens must not be parts of the strings are to be translated:
+- command line option names
+- RPC names


Suggested change

- RPC names

- RPC method and parameter names

luke-jr · 2020-11-24T23:41:52Z

src/init.cpp

+            return InitError(strprintf(_("Prune mode is incompatible with %s."), "-txindex"));
+        }
        if (!g_enabled_filter_types.empty()) {
-            return InitError(_("Prune mode is incompatible with -blockfilterindex."));
+            return InitError(strprintf(_("Prune mode is incompatible with %s."), "-blockfilterindex"));


Should we combine these (like ResolveErrMsg)?

luke-jr · 2020-11-24T23:43:07Z

src/init.cpp


    if (args.IsArgSet("-proxy") && args.GetArg("-proxy", "").empty()) {
-        return InitError(_("No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>."));
+        return InitError(strprintf(_("No proxy server specified. Use %s or %s."), "-proxy=<ip>", "-proxy=<ip:port>"));


It might make sense to translate the =<ip:port> parts?

luke-jr · 2020-11-24T23:43:56Z

src/init.cpp

+            return InitError(strprintf(_("Invalid %s address or hostname: '%s'"), "-proxy", proxyArg));
        }

        proxyType addrProxy = proxyType(proxyAddr, proxyRandomize);
-        if (!addrProxy.IsValid())
-            return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg));
+        if (!addrProxy.IsValid()) {
+            return InitError(strprintf(_("Invalid %s address or hostname: '%s'"), "-proxy", proxyArg));


Another candidate for combining

luke-jr · 2020-11-24T23:44:23Z

src/init.cpp

+                return InitError(strprintf(_("Invalid %s address or hostname: '%s'"), "-onion", onionArg));
            }
            proxyType addrOnion = proxyType(onionProxy, proxyRandomize);
-            if (!addrOnion.IsValid())
-                return InitError(strprintf(_("Invalid -onion address or hostname: '%s'"), onionArg));
+            if (!addrOnion.IsValid()) {
+                return InitError(strprintf(_("Invalid %s address or hostname: '%s'"), "-onion", onionArg));


luke-jr · 2020-11-24T23:46:20Z

src/wallet/wallet.cpp

                if (feeCalc.reason == FeeReason::FALLBACK && !m_allow_fallback_fee) {
                    // eventually allow a fallback fee
-                    error = _("Fee estimation failed. Fallbackfee is disabled. Wait a few blocks or enable -fallbackfee.");
+                    error = strprintf(_("Fee estimation failed. %s is disabled. Wait a few blocks or enable %s."), "-fallbackfee", "-fallbackfee");


First one should probably be translated...

luke-jr · 2020-11-24T23:46:55Z

src/wallet/wallet.cpp

        CAmount nFeePerK = 0;
        if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) {
-            error = strprintf(_("Invalid amount for -discardfee=<amount>: '%s'"), gArgs.GetArg("-discardfee", ""));
+            error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-discardfee", gArgs.GetArg("-discardfee", ""));


This message seems reusable

luke-jr · 2020-11-24T23:47:30Z

src/wallet/wallet.cpp


            if (rescan_height != block_height) {
-                error = _("Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)");
+                error = strprintf(_("Prune: last wallet synchronisation goes beyond pruned data. You need to %s (download the whole block chain again in case of pruned node)."), "-reindex");


This isn't optech

DrahtBot · 2021-02-02T14:13:53Z

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

hebasto · 2021-03-16T23:14:50Z

@laanwj

I understand the idea but… Doesn't this make it much harder for translators to understand what is being translated? A lot of context information goes missing by replacing these with placeholders.

I understand your concerns. Yes, translators will lose some part of context with this approach.
OTOH, for

bitcoin/src/init.cpp

Line 1200 in 7d8a10a

    
           return InitError(_("No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>."));

just spotted the following translation:

(the most common kind of question that I get on transifex is already "I don't understand the context of this message, please explain". It might also result in less specific and relevant translations.)

Does answering those questions resolve translators' lack of context?

laanwj · 2021-03-16T23:41:50Z

Does answering those questions resolve translators' lack of context?

Not really—in the sense that it's not a scalable approach, and also many will not ask and just assume something (often wrongly).
If there is some other way to provide context that is shown in the translator (like the translation comment) I guess that would work though.

hebasto · 2021-03-16T23:51:38Z

If there is some other way to provide context that is shown in the translator (like the translation comment) I guess that would work though.

From https://docs.transifex.com/localization-tips-workflows/the-complete-guide-to-adding-context-to-strings

DEVELOPER NOTES... These types of notes are added to source files by developers. Not all file formats support developer notes though. The file types which support developer notes are the following: Android XML, Apple PLIST, Apple Strings, JSON (Chrome), Structured JSON, PO, Mozilla DTD, Windows Resx, Resw and Resjson, Xliff, xlsx (as context) and YAML.

Qt files are not mentioned, unfortunately.

hebasto · 2021-05-16T23:04:48Z

Closing for now. Leaving up for grabs.

hebasto added 5 commits November 16, 2020 20:42

Do not translate command line option names

8077dc3

Command line option name must not be a part of the translatable string, as it is untranslatable by its nature.

Do not translate RPC names

229f45f

RPC name must not be a part of the translatable string, as it is untranslatable by its nature.

Do not translate debug log file name

7d7c5a4

Drop unneeded '\n' from translatable string

274078b

doc: Update Translation Strings Policy

c20e3db

Added requirement to do not translate names.

DrahtBot added Docs P2P UTXO Db and Indexes Validation Wallet labels Nov 16, 2020

maflcko added GUI and removed P2P UTXO Db and Indexes Validation Wallet labels Nov 17, 2020

decryp2kanon reviewed Nov 18, 2020

View reviewed changes

luke-jr suggested changes Nov 24, 2020

View reviewed changes

DrahtBot mentioned this pull request Nov 26, 2020

Coinstats Index #19521

Merged

This was referenced Dec 11, 2020

Allow maintaining the blockfilterindex when using prune #15946

Merged

wallet, refactor: return out-params of CreateTransaction() as optional struct #20640

Merged

DrahtBot mentioned this pull request Dec 26, 2020

refactor: split CWallet::Create #20773

Merged

DrahtBot mentioned this pull request Jan 23, 2021

wallet: check for non-representable CFeeRates #20546

Closed

DrahtBot added the Needs rebase label Feb 2, 2021

hebasto marked this pull request as draft March 16, 2021 23:54

hebasto closed this May 16, 2021

hebasto mentioned this pull request Jul 21, 2022

refactor: wallet, do not translate init options names #25666

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

	return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of %s."),
	return InitError(strprintf(_("Total length of network version string (%i) exceeds maximum length (%i). Reduce the number or size of %s options."),

Conversation

hebasto commented Nov 16, 2020

Uh oh!

DrahtBot commented Nov 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

decryp2kanon commented Nov 18, 2020

Uh oh!

decryp2kanon Nov 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maflcko commented Nov 18, 2020

Uh oh!

laanwj commented Nov 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Feb 2, 2021

Uh oh!

hebasto commented Mar 16, 2021

Uh oh!

laanwj commented Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Mar 16, 2021

Uh oh!

hebasto commented May 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DrahtBot commented Nov 17, 2020 •

edited

Loading

decryp2kanon Nov 18, 2020 •

edited

Loading

laanwj commented Nov 18, 2020 •

edited

Loading

laanwj commented Mar 16, 2021 •

edited

Loading