Parameterise command line option defaults, so translations are independent of them #5076

luke-jr commented at 9:59 PM on October 10, 2014: member

No description provided.

TheBlueMatt commented at 7:31 AM on October 11, 2014: member

utACK

in src/init.cpp:None in 9b5946dfb0 outdated

 260 | -    strUsage += "  -maxconnections=<n>    " + _("Maintain at most <n> connections to peers (default: 125)") + "\n";
 261 | -    strUsage += "  -maxreceivebuffer=<n>  " + _("Maximum per-connection receive buffer, <n>*1000 bytes (default: 5000)") + "\n";
 262 | -    strUsage += "  -maxsendbuffer=<n>     " + _("Maximum per-connection send buffer, <n>*1000 bytes (default: 1000)") + "\n";
 263 | -    strUsage += "  -onion=<ip:port>       " + _("Use separate SOCKS5 proxy to reach peers via Tor hidden services (default: -proxy)") + "\n";
 264 | +    strUsage += "  -forcednsseed          " + strprintf(_("Always query for peer addresses via DNS lookup (default: %u)"), 0) + "\n";
 265 | +    strUsage += "  -listen                " + strprintf(_("Accept connections from outside (default: %s)"), _("1 if no -proxy or -connect")) + "\n";

laanwj commented at 8:03 AM on October 11, 2014:

Please parametrize only the '1' here, that makes for an easier translation message

luke-jr commented at 8:05 AM on October 11, 2014:

Does it? "0 if no -proxy or -connect" doesn't really make sense :/

laanwj commented at 8:06 AM on October 11, 2014:

See it from this angle: making people translate a message as a fragment that will be inserted as-is into another message is just pointless.

luke-jr commented at 8:08 AM on October 11, 2014:

If the default changes, only that part will be invalidated in the translations, not the entire description?

laanwj commented at 8:09 AM on October 11, 2014:

Why would that be better? In both cases you have to re-translate one message. But in the case of the fragment it makes much less sense to the translator.

in src/init.cpp:None in 9b5946dfb0 outdated

 276 |      strUsage += "  -seednode=<ip>         " + _("Connect to a node to retrieve peer addresses, and disconnect") + "\n";
 277 |      strUsage += "  -timeout=<n>           " + strprintf(_("Specify connection timeout in milliseconds (minimum: 1, default: %d)"), DEFAULT_CONNECT_TIMEOUT) + "\n";
 278 |  #ifdef USE_UPNP
 279 |  #if USE_UPNP
 280 | -    strUsage += "  -upnp                  " + _("Use UPnP to map the listening port (default: 1 when listening)") + "\n";
 281 | +    strUsage += "  -upnp                  " + strprintf(_("Use UPnP to map the listening port (default: %s)"), _("1 when listening")) + "\n";

laanwj commented at 8:05 AM on October 11, 2014:

Same here. Just parametrize the number, not part of the message

luke-jr commented at 8:05 AM on October 11, 2014:

It makes even less sense here to just parameterise the number...

laanwj commented at 8:10 AM on October 11, 2014:

Fine with me - then don't parametrize anything at all here...

laanwj commented at 8:05 AM on October 11, 2014: member

ACK apart from the nits

Parameterise command line option defaults, so translations are independent of them 0a08aa8f2a

luke-jr force-pushed on Oct 11, 2014

Diapolo commented at 5:40 PM on October 11, 2014: none

That is better, but I'm missing constants to replace those default values in the code!

Bugfix: Remove default from -zapwallettxes description (inaccurate) c0195b1c31

in src/init.cpp:None in 0a08aa8f2a outdated

 290 | -    strUsage += "  -wallet=<file>         " + _("Specify wallet file (within data directory)") + " " + _("(default: wallet.dat)") + "\n";
 291 | +    strUsage += "  -wallet=<file>         " + _("Specify wallet file (within data directory)") + " " + strprintf(_("(default: %s)"), "wallet.dat") + "\n";
 292 |      strUsage += "  -walletnotify=<cmd>    " + _("Execute command when a wallet transaction changes (%s in cmd is replaced by TxID)") + "\n";
 293 |      strUsage += "  -zapwallettxes=<mode>  " + _("Delete all wallet transactions and only recover those parts of the blockchain through -rescan on startup") + "\n";
 294 | -    strUsage += "                         " + _("(default: 1, 1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)") + "\n";
 295 | +    strUsage += "                         " + strprintf(_("(default: %u, 1 = keep tx meta data e.g. account owner and payment request information, 2 = drop tx meta data)"), 1) + "\n";

laanwj commented at 9:40 AM on October 14, 2014:

Not your mistake, but I only notice this now: this should be '1 if no <mode> specified'. Let's leave default out of this. If 1 would really be the default, it would always do a zapwallettxes operation!

luke-jr commented at 8:13 PM on October 14, 2014:

Removed

luke-jr commented at 9:28 PM on October 14, 2014: member

Making constants makes more discussion. Let's do that in a follow-up PR after this is merged.

Diapolo commented at 6:22 AM on October 15, 2014: none

Why? Just because you want to hurry ;).

luke-jr commented at 6:27 AM on October 15, 2014: member

Because it's better to do things in small steps so there's less to rebase when one part of it prompts months of discussions. And this gets one improvement out there sooner if the other one pushes past a release. There's nothing inherently related about variablising stuff here.

laanwj commented at 7:11 AM on October 15, 2014: member

Agree with @luke-jr here. Right now this is an extremely low-impact change, and reviewing is limited to the documentation only. If you were to make constants and use those it is more work to verify that everything is unchanged.

laanwj commented at 7:15 AM on October 15, 2014: member

Tested ACK

laanwj merged this on Oct 21, 2014

laanwj closed this on Oct 21, 2014

laanwj referenced this in commit 5f3ea3362a on Oct 21, 2014

luke-jr deleted the branch on Jul 2, 2015

DrahtBot locked this on Sep 8, 2021