qt: Add "Blocksdir" to Debug window

hebasto commented at 8:24 pm on October 2, 2018: member

To get the current blocksdir is valuable for debug purposes after merging #12653.

screenshot from 2018-10-02 23-16-52

fanquake added the label GUI on Oct 2, 2018

Sjors commented at 3:18 am on October 4, 2018: member

utACK 16d79b064c095accc8a3c1decdf54cdd03554564 (see issue below)

Concept ACK

DrahtBot commented at 6:38 am on October 4, 2018: member

Coverage	Change (pull 14374)	Reference (master)
Lines	+0.0331 %	87.0471 %
Functions	+0.1853 %	84.1130 %
Branches	+0.0171 %	51.5403 %

in src/qt/clientmodel.cpp:182 in 16d79b064c outdated

176@@ -177,6 +177,11 @@ QString ClientModel::dataDir() const
177     return GUIUtil::boostPathToQString(GetDataDir());
178 }
179 
180+QString ClientModel::blocksDir() const
181+{
182+    return GUIUtil::boostPathToQString(GetBlocksDir());

promag commented at 0:10 am on October 5, 2018:

Should not call GetBlockDir but get it from interfaces::Node? cc @ryanofsky

ryanofsky commented at 8:49 am on October 8, 2018:

In the long run, promag is right and adding an interfaces::Node method would be better, but for now calling GetBlocksDir() directly is more consistent with existing code. The goal of adding the Node and Wallet interfaces in #10244 was to get rid of accesses to non-GUI global variables from GUI code. But to make the change more minimal, I exempted gArgs and chainparams variables, and instead added some glue code in #10102 to let each process just keep its own copies of these variables. As a result, it’s fine to call functions like GetBlocksDir and GetDataDir from GUI code that access them.

promag commented at 0:14 am on October 5, 2018: member

Concept ACK. Could have a way to tell if it’s the default blocks directory or user supplied.

Sjors commented at 4:27 am on October 5, 2018: member

@promag and/or a tooltip that explains this location can be customized using -blocksdir

Agree we need to clarify if GetBlockDir should be avoided in favor of adding a method to interfaces::Node.

hebasto commented at 7:52 pm on October 5, 2018: member

@Sjors @promag Thank you for your reviews.

Should not call GetBlockDir but get it from interfaces::Node?

Agree we need to clarify if GetBlockDir should be avoided in favor of adding a method to interfaces::Node.

Besides GetBlocksDir the ClientModel calls GetDataDir, GetStartupTime and GetTimeMillis as well. All are the thread-safe utilities not from the node code. IMO, using the GetBlocksDir is a safe way.

Nevertheless, I agree with all of you: @ryanofsky’s review will be much appreciated.

Add "Blocksdir" to Debug window

To get the current blocksdir is valuable for debug purposes after
merging #12653.

3045704502

hebasto force-pushed on Oct 5, 2018

hebasto commented at 9:22 pm on October 5, 2018: member

a tooltip that explains this location can be customized using -blocksdir

Done:

screenshot from 2018-10-06 00-15-15

in src/qt/forms/debugwindow.ui:131 in bde394669f outdated

126@@ -127,6 +127,9 @@
127          <property name="cursor">
128           <cursorShape>IBeamCursor</cursorShape>
129          </property>
130+         <property name="toolTip">
131+          <string>&lt;html&gt;&lt;head/&gt;&lt;body&gt;&lt;p&gt;To specify a non-default location of the data directory use the '&amp;#8209;datadir' option.&lt;/p&gt;&lt;/body&gt;&lt;/html&gt;</string>

Sjors commented at 8:24 am on October 8, 2018:

What’s with the <html><head/><body><p>? This probably makes the tooltip hard to translate.

hebasto commented at 8:43 am on October 8, 2018:

It uses HTML code ‑ for the non-breaking hyphen character. Otherwise, the tooltip can looks ugly. This LOC has been generated by Qt Creator and, therefore, the translation should be treated in a correct way. Or did I miss something?

Sjors commented at 8:56 am on October 8, 2018:

The &#8209; bit seems fine, but I was talking about the html etc prefix. I think most developers use a text editor instead of the Qt Creator tools. Perhaps @laanwj knows if this works with the translation system?

ryanofsky commented at 9:02 am on October 8, 2018:

Can you just use the utf8-encoding of the chr(8209) character? ‑? Presumably then you wouldn’t need the html stuff.

Sjors changes_requested

Sjors commented at 8:26 am on October 8, 2018: member

tACK bde3946 modulo the tooltip html.

ryanofsky approved

ryanofsky commented at 9:04 am on October 8, 2018: member

utACK bde394669ff5dc666d8b8ad1c0e74244a427eec0

Add tooltips for both datadir and blocksdir 2ab9140c92

hebasto force-pushed on Oct 8, 2018

hebasto commented at 10:40 am on October 8, 2018: member

@Sjors

This probably makes the tooltip hard to translate.

You are right. I’ve missed this warning Avoid HTML in translation strings @ryanofsky Thank you for your review.

Tooltips are fixed. Please re-review.

ryanofsky approved

ryanofsky commented at 1:44 am on October 10, 2018: member

utACK 2ab9140c92c7ffd950f9ea6e1e78107a217bb336 but I think it would be better if you just literally wrote the nonbreaking hyphen in the XML as a utf8-encoded character (as suggested previously):

0-          <string>To specify a non-default location of the data directory use the '%1' option.</string>
1+          <string>To specify a non-default location of the data directory use the '‑datadir' option.</string>
2-          <string>To specify a non-default location of the blocks directory use the '%1' option.</string>
3+          <string>To specify a non-default location of the blocks directory use the '‑blocksdir' option.</string>

Sjors commented at 2:47 am on October 10, 2018: member

Actually I like the %1 approach better. It prevents translators from accidentally breaking the - or worse, translating the actual command.

utACK 2ab9140

jonasschnelli commented at 6:51 pm on October 17, 2018: contributor

Tested a bit. I think we should only display this when the blocksdir is not the default.

sipa commented at 11:27 pm on October 17, 2018: member

Concept ACK

laanwj merged this on Oct 18, 2018

laanwj closed this on Oct 18, 2018

laanwj referenced this in commit fe23553edd on Oct 18, 2018

hebasto deleted the branch on Oct 18, 2018

Munkybooty referenced this in commit 6b87e6e5db on Jul 21, 2021

Munkybooty referenced this in commit e2043dc2e9 on Jul 21, 2021

Munkybooty referenced this in commit 29c0104010 on Jul 22, 2021

Munkybooty referenced this in commit b70b4f01b7 on Jul 22, 2021

Munkybooty referenced this in commit 8c71bc1f3c on Jul 23, 2021

DrahtBot locked this on Sep 8, 2021

qt: Add “Blocksdir” to Debug window #14374