When wallet is enabled -help shows the info about passing an URI to the bitcoin-qt:
$ src/qt/bitcoin-qt -? | head
Bitcoin Core version v21.99.0-82f37decdbba
Usage: bitcoin-qt [options] [URI]
URI must follow BIP 21
Options:
-?
Print this help message and exit
ACK 82f37decdbba340ea9798e0c82ae7b3cc4100248
I did not know you can provide a URI when running bitcoin-qt, so this is definitely helpful information.
One thing is that it would leave the user wondering if they aren't already aware, "what is BIP 21?" An internet search suffices, but what If a hyper-link was used?
... what If a hyper-link was used?
Not sure if it is worth to add complexity, but I will do it if there will be reviewers' consensus.
509 | @@ -510,7 +510,11 @@ int GuiMain(int argc, char* argv[]) 510 | // Show help message immediately after parsing command-line options (for "-lang") and setting locale, 511 | // but before showing splash screen. 512 | if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { 513 | - HelpMessageDialog help(nullptr, gArgs.IsArgSet("-version")); 514 | + bool wallet_enabled{false};
nit: welp, we could go full const by using complete set of #ifdef / #else / #endif:
#ifdef ENABLE_WALLET
const bool wallet_enabled{WalletModel::isWalletEnabled()};
#else
constexpr bool wallet_enabled{false};
#endif // ENABLE_WALLET
21 | @@ -22,7 +22,7 @@ class HelpMessageDialog : public QDialog 22 | Q_OBJECT 23 | 24 | public: 25 | - explicit HelpMessageDialog(QWidget *parent, bool about); 26 | + explicit HelpMessageDialog(QWidget *parent, bool about, bool wallet_enabled = true);
Not sure if we need = true default. HelpMessageDialog is not something that is used a lot (only twice?), and if it was, assuming = true as default would probably be error prone, as it has to actually depend on the build.
Also, bitcoingui.h has bool enableWallet = false; defined, so at least it's inconsistency.
btw, I think that combining two independent dialogs in one widget is a bad implementation.
I mean about parameter.
57 | @@ -58,7 +58,8 @@ HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) : 58 | ui->helpMessage->setVisible(false); 59 | } else { 60 | setWindowTitle(tr("Command-line options")); 61 | - QString header = "Usage: bitcoin-qt [command-line options] \n"; 62 | + const QString uri_info = wallet_enabled ? QStringLiteral(" [URI]\n\nURI must follow BIP 21") : QString(); 63 | + const QString header(QLatin1String("Usage: bitcoin-qt [options]") + uri_info + QLatin1Char('\n'));
nit: well, if care was taken to use lean QLatin1String, including <QStringBuilder> and using % instead of + would make it even beter. Using QStringBuilder avoids that quadratic re-copying of string buffer with every +.
https://wiki.qt.io/Using_QString_Effectively#QStringBuilder_:_Fast_QString_concatenation
https://doc.qt.io/qt-5/qstring.html#more-efficient-string-construction
Concept ACK
509 | @@ -510,7 +510,12 @@ int GuiMain(int argc, char* argv[]) 510 | // Show help message immediately after parsing command-line options (for "-lang") and setting locale, 511 | // but before showing splash screen. 512 | if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { 513 | - HelpMessageDialog help(nullptr, gArgs.IsArgSet("-version")); 514 | +#ifdef ENABLE_WALLET 515 | + const bool wallet_enabled{WalletModel::isWalletEnabled()};
Whether this running instance has the wallet disabled at runtime or not is irrelevant to a new execution... Can fold the #ifdef deep into utilitydialog.cpp
Correct. The help message is about the build, not the current runtime.
21 | @@ -22,7 +22,7 @@ class HelpMessageDialog : public QDialog 22 | Q_OBJECT 23 | 24 | public: 25 | - explicit HelpMessageDialog(QWidget *parent, bool about); 26 | + explicit HelpMessageDialog(QWidget* parent, bool about, bool wallet_enabled = false);
I see default changed to = false, but do we really need it (default value) at all?
I mean, why some "abstract" user of this class would allow himself to skip passing explicit argument and let default be assumed as false? Maybe there's argument for it and I am missing it.
When about is true it does not matter what value wallet_enabled has. It seems weird to choose any its value on the caller site.
Oh, so it depends on about too. OK got it.
58 | @@ -58,7 +59,8 @@ HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) : 59 | ui->helpMessage->setVisible(false); 60 | } else { 61 | setWindowTitle(tr("Command-line options")); 62 | - QString header = "Usage: bitcoin-qt [command-line options] \n"; 63 | + const QString uri_info = wallet_enabled ? QStringLiteral(" [URI]\n\nURI must follow BIP 21") : QString();
The URI has to be enclosed in single/double quotation marks. Maybe this should be added to avoid people wondering why they get to an exit message in their terminal.
const QString uri_info = wallet_enabled ? QStringLiteral(" ["URI"]\n\nURI must follow BIP 21") : QString();
Closing and leaving it up for grabs.