Add a button to open the config file in a text editor #9890

ghost commented at 5:17 PM on February 28, 2017: none

This button was described in issue #9655

I added it to the options dialog

fanquake added the label GUI on Mar 1, 2017

laanwj commented at 10:57 AM on March 1, 2017: member

Concept ACK

jonasschnelli commented at 12:18 PM on March 1, 2017: contributor

Nice. Concept ACK. Will test soon.

jonasschnelli commented at 9:30 PM on March 1, 2017: contributor

I think it should create the file if bitcoin.conf does not exists (right now, there is no reaction if the file does not exists)
The buttons should probably aligned/distributed vertically (horizontally it will eat up a lot of space in non en_EN lang)

ghost commented at 10:00 PM on March 1, 2017: none

@jonasschnelli Ok

ghost commented at 2:11 PM on March 2, 2017: none

@jonasschnelli Is this what you had in mind for the layout? I wanted to check before pushing

button_demo

jonasschnelli commented at 2:53 PM on March 2, 2017: contributor

@ericshawlinux: I think this looks better and is more flexible.

ghost commented at 3:48 PM on March 2, 2017: none

I think so too. I amended my commit from the other day

ghost commented at 6:09 PM on March 2, 2017: none

(I just fixed a mistake I made, I accidentally misplaced a spacer while using Qt-designer. It is back where it belongs now)

luke-jr commented at 3:41 AM on March 3, 2017: member

I wonder if users might get configured that none of their options show up in the config file...? Not really sure how one might improve on this, though.

jonasschnelli commented at 10:30 AM on March 3, 2017: contributor

I wonder if users might get configured that none of their options show up in the config file...? Not really sure how one might improve on this, though.

Yes. We need to watch out here. The configuration levels are complex.

startup arguments (-arg=val)
bitcoin.conf
Qt configuration (GUI)

If a user opens "the" configuration file, he might expect to see all existing configuration values (including the GUI and startup one).

We should probably label the button "open experts configuration file" or similar.

ghost commented at 1:11 PM on March 3, 2017: none

Well it says "Active command-line options override the above options" so maybe if we put the new button above that, the user will know they are different.

jonasschnelli commented at 1:44 PM on March 3, 2017: contributor

An alternative would be to first show an QMessageBox with some infos about how the bitcoin.conf works and after that alert box, open the bitcoin.conf file.

ghost commented at 3:32 PM on March 3, 2017: none

Okay. How does this sound? "The configuration is used to specify advanced user options less any command-line or Qt options. Any command-line options will override this configuration file."

ghost commented at 6:28 PM on March 7, 2017: none

I added the information dialog

in src/qt/guiutil.cpp:425 in 1c6c2b7876 outdated

 416 | @@ -417,6 +417,18 @@ void openDebugLogfile()
 417 |          QDesktopServices::openUrl(QUrl::fromLocalFile(boostPathToQString(pathDebug)));
 418 |  }
 419 |  
 420 | +void openBitcoinConf()
 421 | +{
 422 | +    boost::filesystem::path pathConfig = GetConfigFile(BITCOIN_CONF_FILENAME);
 423 | +
 424 | +    /* Create the file */
 425 | +    std::fstream fs(pathConfig.c_str(), std::fstream::app);

jonasschnelli commented at 3:11 PM on March 17, 2017:

Gitian Windows Build is failing here:

qt/guiutil.cpp: In function ‘void GUIUtil::openBitcoinConf()’:
qt/guiutil.cpp:425:58: error: no matching function for call to ‘std::basic_fstream<char>::basic_fstream(const value_type*, const openmode&)’
     std::fstream fs(pathConfig.c_str(), std::fstream::app);
                                                          ^
qt/guiutil.cpp:425:58: note: candidates are:
In file included from /home/ubuntu/build/bitcoin/depends/i686-w64-mingw32/share/../include/boost/filesystem/operations.hpp:45:0,
                 from /home/ubuntu/build/bitcoin/depends/i686-w64-mingw32/share/../include/boost/filesystem.hpp:17,
                 from qt/guiutil.h:19,
                 from qt/guiutil.cpp:5:
/usr/include/c++/4.8/fstream:831:7: note: std::basic_fstream<_CharT, _Traits>::basic_fstream(const string&, std::ios_base::openmode) [with _CharT = char; _Traits = std::char_traits<char>; std::string = std::basic_string<char>; std::ios_base::openmode = std::_Ios_Openmode]
       basic_fstream(const std::string& __s,

unknown commented at 12:13 AM on March 18, 2017:

ok, I can look at it on monday

jonasschnelli commented at 3:18 PM on March 17, 2017: contributor

Currently the build is failing on Windows (over Gitian): https://bitcoin.jonasschnelli.ch/build/53

Otherwise this works fine. Tested on OSX and Ubuntu.

ghost commented at 3:27 PM on March 20, 2017: none

okay, I had an issue with git for a few minutes, but I think I fixed it now.

as for the code, I changed the mode from std::fstream::app to std::ios_base::app that builds okay with linux, and I think with windows, since I read the error message you sent me.

jonasschnelli commented at 3:38 PM on March 20, 2017: contributor

Thanks... started again a gitian build: https://bitcoin.jonasschnelli.ch/build/60

ghost commented at 4:48 PM on March 20, 2017: none

Still didn't work. I guess I didn't look closely enough at the error before. Now, I think I need to call path.native() instead of path.c_str() based on the constructor declaration.

I will push that soon

laanwj commented at 5:02 PM on March 20, 2017: member

Please use qstringToBoostPath and boostPathToQString from GUIUtil to convert between QString and boost::filesystem. They avoid UTF-8 issues that will arise when using other methods.

ghost commented at 5:11 PM on March 20, 2017: none

ok

ghost commented at 6:08 PM on March 20, 2017: none

thanks, I figured it out using boost and didn't need to convert the path there. It should build on windows now

ghost commented at 7:24 PM on March 20, 2017: none

@jonasschnelli could you do the gitian build on your site again? I am pretty confident it will work this time after Laan advised me, I changed it to use boost path and not a c string.

laanwj commented at 7:38 PM on March 20, 2017: member

Code looks good to me now, although I wonder about error handling: what happens if the config file is not writable?

ghost commented at 8:18 PM on March 20, 2017: none

When I change the permissions, It asks me how I want to open the file, which doesn't make sense. I can add some logic and show a QMessageBox::critical() dialog if it fails to open the file.

ghost commented at 1:32 PM on March 21, 2017: none

Now an error appears when the file cannot be opened.

error2

fanquake commented at 11:25 PM on March 22, 2017: member

Testing 541f8d8 on OS X. Appearance in the GUI: screen shot 1 Pre opening message: screen shot 2 Unable to open file error message: screen shot 4

I'm wondering if we can word the pre-opening message any better. Could we use Core rather than qt, or is that more confusing?

MarcoFalke commented at 11:06 AM on March 23, 2017: member

Maybe replace 'qt options' with 'gui settings'?

laanwj commented at 2:58 PM on March 23, 2017: member

Some comments on messages:

Agree with @MarcoFalke , "Qt options" should be "GUI settings". Except when directly referring to the widget toolkit we avoid referring to Qt, as that's an implementation detail.
"config file" in the error message should be "configuration file" (as the button) or just the filename, "bitcoin.conf". There's no reason to shorten here, and it could make it more difficult for translators.
Instead of "less any..." I'd use "except any...". Seems more common English (at least to me as non-native speaker.)

ghost commented at 4:44 PM on March 23, 2017: none

I agree, saying "less any..." is not common, but to make it more descriptive and clear I reworded the whole message the best that I could. How is this? "The configuration file is used to specify advanced user options which override GUI settings. Additionally, any command-line options will override this configuration file."

Add a button to open the config file in a text editor 9ab9e7d1b3

jonasschnelli commented at 11:47 AM on March 27, 2017: contributor

Tested ACK 9ab9e7d1 (https://bitcoin.jonasschnelli.ch/build/69)

jonasschnelli merged this on Apr 10, 2017

jonasschnelli closed this on Apr 10, 2017

jonasschnelli referenced this in commit 1fa4ae67a3 on Apr 10, 2017

luke-jr referenced this in commit 69e51bc775 on Apr 21, 2017

MarcoFalke locked this on Sep 8, 2021