Fix #15409. The QT wallet fail to open the configuration file on Mac, when these is no default application for *.conf files.
Here is a feasible way to solve this bug. When QDesktopServices::openUrl fails to open file:///path/bitcoin.conf with its default application, use QProcess::startDetached to run open -t /path/bitcoin.conf command instead, so as to open the configuration file with system's default text editor.
to run "open -t qtum.conf
What is qtum.conf ?
Sorry. Since I find this bug initially in the Qtum blockchain, I wrote it wrong and have changed the text...
The mentioned issue #15409 is related to the setting a default application to open *.conf files.
This can be easily achieved via macOS GUI.
/usr/bin/open -t forces to open a file with default text editor. This implies the setting a default application to open *.conf files will not work, right?
@shannon1916 your PR description could be edited using this GitHub tips ;)
@hebasto The PR description is modified as you suggest.
And, /usr/bin/open -t only works when QDesktopServices::openUrl fails, as detailed in the PR description.
Concept ACK. Will test.
tACK 57b30ce16cf6d4d2e6a30e70ede7f2bdd5fbc003 modulo nits (macOS 10.13.6)
As my system already has a default app to open *.conf files, the next patch was applied for testing purpose:
diff --git a/src/util/system.cpp b/src/util/system.cpp
index 6925bda4e..a5e24c0f3 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -72 +72 @@ const int64_t nStartupTime = GetTime();
-const char * const BITCOIN_CONF_FILENAME = "bitcoin.conf";
+const char * const BITCOIN_CONF_FILENAME = "bitcoin.conftest";
399 | @@ -399,7 +400,13 @@ bool openBitcoinConf() 400 | configFile.close(); 401 | 402 | /* Open bitcoin.conf with the associated application */ 403 | - return QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 404 | + bool res = QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 405 | +#if defined(Q_OS_MAC) 406 | + if (!res) 407 | + res = QProcess::startDetached("/usr/bin/open", QStringList() << "-t" << boostPathToQString(pathConfig));
nit: could if statement be placed in braces?
micronit: using an initializer list is shorter:
QStringList{"-t", boostPathToQString(pathConfig)}
fixed in the new commit
399 | @@ -399,7 +400,13 @@ bool openBitcoinConf() 400 | configFile.close(); 401 | 402 | /* Open bitcoin.conf with the associated application */ 403 | - return QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 404 | + bool res = QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 405 | +#if defined(Q_OS_MAC)
nit: #ifdef is more common in Bitcoin Core code base ;)
fixed in the new commit
@hebasto The default app for *.conf files can be set in System Preferences -> Default Apps -> Extensions, as shown below
Too bad this required a MAC #ifdef and a fixed path to an executable. But I guess it's an acceptable fix. utACK 0ebaa3ad8c252e8d6da37ac62ed568237bbb4bad
399 | @@ -399,7 +400,14 @@ bool openBitcoinConf() 400 | configFile.close(); 401 | 402 | /* Open bitcoin.conf with the associated application */ 403 | - return QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 404 | + bool res = QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 405 | +#ifdef Q_OS_MAC 406 | + if (!res) {
I think this needs a comment why this fallback is necessary.
A comment like https://github.com/bitcoin/bitcoin/blob/master/src/qt/guiutil.cpp#L944 ?
Like // Workaround for macOS-only Qt bug; see: ISSUE [#15409](/bitcoin-bitcoin/15409/)?
@shannon1916 A comment like // Workaround for macOS-only Qt bug; see [#15409](/bitcoin-bitcoin/15409/) should be ok. Once you've added it can you squash all of your commits.
IMO, this issue is not a Qt bug. This PR addresses macOS-specific behavior.
May I suggest a comment // Workaround for macOS-specific behavior; see [#15409](/bitcoin-bitcoin/15409/). ?
Done
Concept ACK
Have checked that this change works on macOS with and without a .conf file already in place.
In the later case a blank .conf is created and opened.
Will re-ack post comment fix-up and squash.
re-ACK 6e6494b3fb345848025494cb7a79c5bf8f35e417
tACK https://github.com/bitcoin/bitcoin/commit/6e6494b3fb345848025494cb7a79c5bf8f35e417 on macOS 10.14.x
Using master (c7cfd20a77ce57d200b3b9e5e0dfb0d63818abdc) you cannot open bitcoin.conf through the preferences menu:
Using this PR (6e6494b3fb345848025494cb7a79c5bf8f35e417):
Opening an existing bitcoin.conf works, and if one doesn't exist, a blank one is created and opened.
lldb Bitcoin-Qt.app -- -regtest
(lldb) target create "Bitcoin-Qt.app"
Current executable set to 'Bitcoin-Qt.app' (x86_64).
(lldb) settings set -- target.run-args "-regtest"
(lldb) run
Process 40791 launched: '/Users/michael/github/bitcoin/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt' (x86_64)
2019-06-03 10:00:00.852544-0400 Bitcoin-Qt[40791:8481788] MessageTracer: Falling back to default whitelist
2019-06-03 10:00:07.232235-0400 open[40799:8481923] MessageTracer: Falling back to default whitelist
2019-06-03 10:00:19.079914-0400 open[40928:8482455] MessageTracer: Falling back to default whitelist
399 | @@ -399,7 +400,15 @@ bool openBitcoinConf() 400 | configFile.close(); 401 | 402 | /* Open bitcoin.conf with the associated application */ 403 | - return QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 404 | + bool res = QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathConfig))); 405 | +#ifdef Q_OS_MAC 406 | + // Workaround for macOS-specific behavior; see #15409. 407 | + if (!res) { 408 | + res = QProcess::startDetached("/usr/bin/open", QStringList{"-t", boostPathToQString(pathConfig)});
Just "open"?
high-sierra:~ hebasto$ /usr/bin/open
Usage: open [-e] [-t] [-f] [-W] [-R] [-n] [-g] [-h] [-s <partial SDK name>][-b <bundle identifier>] [-a <application>] [filenames] [--args arguments]
Help: Open opens files from a shell.
By default, opens each file using the default application for that file.
If the file is in the form of a URL, the file will be opened as a URL.
Options:
-a Opens with the specified application.
-b Opens with the specified application bundle identifier.
-e Opens with TextEdit.
-t Opens with default text editor.
-f Reads input from standard input and opens with TextEdit.
-F --fresh Launches the app fresh, that is, without restoring windows. Saved persistent state is lost, excluding Untitled documents.
-R, --reveal Selects in the Finder instead of opening.
-W, --wait-apps Blocks until the used applications are closed (even if they were already running).
--args All remaining arguments are passed in argv to the application's main() function instead of opened.
-n, --new Open a new instance of the application even if one is already running.
-j, --hide Launches the app hidden.
-g, --background Does not bring the application to the foreground.
-h, --header Searches header file locations for headers matching the given filenames, and opens them.
-s For -h, the SDK to use; if supplied, only SDKs whose names contain the argument value are searched.
Otherwise the highest versioned SDK in each platform is used.
The explicit path seems better, right?
As this is an OS specific hack, I think hardcoding the path seems fine, it's always that.
Milestone
0.18.1