qt: Add "Blocksdir" to Debug window #14374

hebasto · 2018-10-02T20:24:17Z

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

Sjors · 2018-10-04T03:18:26Z

~~utACK 16d79b064c095accc8a3c1decdf54cdd03554564~~ (see issue below)

Concept ACK

DrahtBot · 2018-10-04T06:38:38Z

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

promag

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

promag · 2018-10-05T00:10:37Z

src/qt/clientmodel.cpp

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

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.

Sjors · 2018-10-05T04:27:45Z

@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 · 2018-10-05T19:52:24Z

@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.

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

hebasto · 2018-10-05T21:22:10Z

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

Done:

Sjors

tACK bde3946 modulo the tooltip html.

Sjors · 2018-10-08T08:24:01Z

src/qt/forms/debugwindow.ui

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

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?

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?

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

ryanofsky

utACK bde394669ff5dc666d8b8ad1c0e74244a427eec0

ryanofsky · 2018-10-08T09:02:10Z

src/qt/forms/debugwindow.ui

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

hebasto · 2018-10-08T10:40:56Z

@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

utACK 2ab9140 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):

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

Sjors · 2018-10-10T02:47:36Z

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

utACK 2ab9140

jonasschnelli · 2018-10-17T18:51:03Z

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

sipa · 2018-10-17T23:27:46Z

Concept ACK

2ab9140 Add tooltips for both datadir and blocksdir (Hennadii Stepanov) 3045704 Add "Blocksdir" to Debug window (Hennadii Stepanov) Pull request description: To get the current `blocksdir` is valuable for debug purposes after merging #12653. ![screenshot from 2018-10-02 23-16-52](https://user-images.githubusercontent.com/32963518/46374770-2ef6f580-c69a-11e8-85c2-44a49fa36b28.png) Tree-SHA512: a93f2c00ee19cf6acb499d3bd9bccf4be8ef01c53c44d917ad401aa4797db02cbccb71a9c24e05262ea09345e15f9299381367fdc6951f21dd3788a4a58d2132

2ab9140 Add tooltips for both datadir and blocksdir (Hennadii Stepanov) 3045704 Add "Blocksdir" to Debug window (Hennadii Stepanov) Pull request description: To get the current `blocksdir` is valuable for debug purposes after merging bitcoin#12653. ![screenshot from 2018-10-02 23-16-52](https://user-images.githubusercontent.com/32963518/46374770-2ef6f580-c69a-11e8-85c2-44a49fa36b28.png) Tree-SHA512: a93f2c00ee19cf6acb499d3bd9bccf4be8ef01c53c44d917ad401aa4797db02cbccb71a9c24e05262ea09345e15f9299381367fdc6951f21dd3788a4a58d2132

fanquake added the GUI label Oct 2, 2018

promag reviewed Oct 5, 2018

View reviewed changes

Add "Blocksdir" to Debug window

3045704

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

hebasto force-pushed the 20181002-debugwindow-blocksdir branch from 16d79b0 to bde3946 Compare October 5, 2018 21:19

Sjors suggested changes Oct 8, 2018

View reviewed changes

ryanofsky approved these changes Oct 8, 2018

View reviewed changes

Add tooltips for both datadir and blocksdir

2ab9140

hebasto force-pushed the 20181002-debugwindow-blocksdir branch from bde3946 to 2ab9140 Compare October 8, 2018 10:36

ryanofsky approved these changes Oct 10, 2018

View reviewed changes

laanwj merged commit 2ab9140 into bitcoin:master Oct 18, 2018

hebasto deleted the 20181002-debugwindow-blocksdir branch October 18, 2018 12:12

promag mentioned this pull request Dec 11, 2018

Migrate chainstate to SSD, or load it to memory #14904

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

qt: Add "Blocksdir" to Debug window #14374

qt: Add "Blocksdir" to Debug window #14374

Uh oh!

Conversation

hebasto commented Oct 2, 2018

Uh oh!

Sjors commented Oct 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 4, 2018

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Oct 5, 2018

Choose a reason for hiding this comment

Uh oh!

ryanofsky Oct 8, 2018

Choose a reason for hiding this comment

Uh oh!

Sjors commented Oct 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Oct 5, 2018

Uh oh!

hebasto commented Oct 5, 2018

Uh oh!

Sjors left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors Oct 8, 2018

Choose a reason for hiding this comment

Uh oh!

hebasto Oct 8, 2018

Choose a reason for hiding this comment

Uh oh!

Sjors Oct 8, 2018

Choose a reason for hiding this comment

Uh oh!

ryanofsky Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Oct 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Sjors commented Oct 10, 2018

Uh oh!

jonasschnelli commented Oct 17, 2018

Uh oh!

sipa commented Oct 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Sjors commented Oct 4, 2018 •

edited

Loading

Sjors commented Oct 5, 2018 •

edited

Loading

ryanofsky Oct 8, 2018 •

edited

Loading

ryanofsky Oct 8, 2018 •

edited

Loading

hebasto commented Oct 8, 2018 •

edited

Loading