Three follow-ups to #163:
- return relay type for inbound peers
- improve markup handling in the tooltip to facilitate translations
- update ConnectionType doxygen documentation
Peer details: connection type follow-ups #180
pull jonatack wants to merge 3 commits into bitcoin-core:master from jonatack:peer-details-connection-type-followups changing 5 files +26 −13-
jonatack commented at 1:56 pm on January 10, 2021: contributor
-
in src/qt/rpcconsole.cpp:475 in 09f4414509 outdated
471+ "<li>Outbound Full Relay: default</li>" 472+ "<li>Outbound Block Relay: does not relay transactions or addresses</li>" 473+ "<li>Outbound Manual: added using RPC %1 or %2/%3 configuration options</li>" 474+ "<li>Outbound Feeler: short-lived, for testing addresses</li>" 475+ "<li>Outbound Address Fetch: short-lived, for soliciting addresses</li>" 476+ "</ul>")
jonatack commented at 2:00 pm on January 10, 2021:I’m not sure if the markup should be replaced by placeholders here or not. I see inline *.cpp markup liketr("<b>abc</b>")elsewhere in the GUI codebase.
jonatack commented at 5:28 pm on January 10, 2021:resolvedin src/qt/rpcconsole.cpp:478 in 09f4414509 outdated
474+ "<li>Outbound Feeler: short-lived, for testing addresses</li>" 475+ "<li>Outbound Address Fetch: short-lived, for soliciting addresses</li>" 476+ "</ul>") 477+ .arg("addnode") 478+ .arg(QString(nonbreaking_hyphen) + "addnode") 479+ .arg(QString(nonbreaking_hyphen) + "connect")));
hebasto commented at 3:42 pm on January 10, 2021:How about
0--- a/src/qt/rpcconsole.cpp 1+++ b/src/qt/rpcconsole.cpp 2@@ -463,19 +463,19 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty 3 ui->dataDir->setToolTip(ui->dataDir->toolTip().arg(QString(nonbreaking_hyphen) + "datadir")); 4 ui->blocksDir->setToolTip(ui->blocksDir->toolTip().arg(QString(nonbreaking_hyphen) + "blocksdir")); 5 ui->openDebugLogfileButton->setToolTip(ui->openDebugLogfileButton->toolTip().arg(PACKAGE_NAME)); 6- ui->peerConnectionTypeLabel->setToolTip( 7- ui->peerConnectionTypeLabel->toolTip() 8- .arg(tr("<ul>" 9- "<li>Inbound Relay: initiated by peer</li>" 10- "<li>Outbound Full Relay: default</li>" 11- "<li>Outbound Block Relay: does not relay transactions or addresses</li>" 12- "<li>Outbound Manual: added using RPC %1 or %2/%3 configuration options</li>" 13- "<li>Outbound Feeler: short-lived, for testing addresses</li>" 14- "<li>Outbound Address Fetch: short-lived, for soliciting addresses</li>" 15- "</ul>") 16- .arg("addnode") 17- .arg(QString(nonbreaking_hyphen) + "addnode") 18- .arg(QString(nonbreaking_hyphen) + "connect"))); 19+ 20+ const std::vector<QString> list_items{ 21+ tr("Inbound Relay: initiated by peer"), 22+ tr("Outbound Full Relay: default"), 23+ tr("Outbound Block Relay: does not relay transactions or addresses"), 24+ tr("Outbound Manual: added using RPC %1 or %2/%3 configuration options") 25+ .arg("addnode") 26+ .arg(QString(nonbreaking_hyphen) + "addnode") 27+ .arg(QString(nonbreaking_hyphen) + "connect"), 28+ tr("Outbound Feeler: short-lived, for testing addresses"), 29+ tr("Outbound Address Fetch: short-lived, for soliciting addresses")}; 30+ const QString list = "<ul><li>" + Join(list_items, QString("</li><li>")) + "</li></ul>"; 31+ ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(list)); 32 33 if (platformStyle->getImagesOnButtons()) { 34 ui->openDebugLogfileButton->setIcon(platformStyle->SingleColorIcon(":/icons/export"));?
(and #include ~<string.h>~ <util/string.h> )
jonatack commented at 5:01 pm on January 10, 2021:Nice, thanks! We were thinking in the same direction. Adapted it and added your name to the commit.jonatack force-pushed on Jan 10, 2021in src/qt/rpcconsole.cpp:48 in 3bb3ad2223 outdated
44@@ -45,6 +45,7 @@ 45 #include <QTime> 46 #include <QTimer> 47 48+#include <string>
hebasto commented at 5:04 pm on January 10, 2021:My bad, I meant#include <util/string.h>for theJointemplate function.
jonatack commented at 5:09 pm on January 10, 2021:Oops, fixedin src/qt/rpcconsole.cpp:464 in 3bb3ad2223 outdated
459@@ -459,11 +460,22 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty 460 461 ui->splitter->restoreState(settings.value("PeersTabSplitterSizes").toByteArray()); 462 463- QChar nonbreaking_hyphen(8209); 464+ const QChar nonbreaking_hyphen(8209); 465+ const std::vector<QString> CONNECTION_TYPE_DOC{
jonatack commented at 5:08 pm on January 10, 2021:Named itCONNECTION_TYPE_DOCto show up in git grep with the constant of the same name inrpc/net.cppand updated thenet.hdocumentationjonatack force-pushed on Jan 10, 2021jonatack force-pushed on Jan 10, 2021in src/qt/guiutil.cpp:770 in 1a297a7ea0 outdated
763@@ -764,10 +764,10 @@ QString NetworkToQString(Network net) 764 assert(false); 765 } 766 767-QString ConnectionTypeToQString(ConnectionType conn_type) 768+QString ConnectionTypeToQString(ConnectionType conn_type, bool relay_txes) 769 { 770 switch (conn_type) { 771- case ConnectionType::INBOUND: return QObject::tr("Inbound"); 772+ case ConnectionType::INBOUND: return QObject::tr(relay_txes ? "Inbound Full Relay" : "Inbound Block Relay");
hebasto commented at 5:44 pm on January 10, 2021:df5fb7ed64f5e47cad66c2fe3582c66a7896f818, for correct work of translation mechanics
0 case ConnectionType::INBOUND: return relay_txes ? QObject::tr("Inbound Full Relay") : QObject::tr("Inbound Block Relay");
jonatack commented at 6:10 pm on January 10, 2021:It seems equivalent? Anyway, done–thank you for looking!
hebasto commented at 6:56 pm on January 10, 2021:It is not equivalent for the
lupdatetool:When Qt’s translation tool, lupdate, is used to process a set of source files, the text wrapped in
tr()calls is stored in a section of the translation file that corresponds to its translation context.hebasto changes_requestedhebasto commented at 5:45 pm on January 10, 2021: memberApproach ACK 1a297a7ea08c3ca5c591ba4942a5a12bc90a210d.jonatack force-pushed on Jan 10, 2021in src/net.h:115 in e711200759 outdated
110@@ -111,7 +111,8 @@ struct CSerializedNetMsg 111 * connection. Aside from INBOUND, all types are initiated by us. 112 * 113 * If adding or removing types, please update CONNECTION_TYPE_DOC in 114- * src/rpc/net.cpp. */ 115+ * src/rpc/net.cpp and src/qt/rpcconsole.cpp, as well as the descriptions in 116+ * src/qt/guiutil.cpp and src/bitcoin-cli.cpp::NetinfoRequestHandler. */
hebasto commented at 7:47 pm on January 10, 2021:nit: A compiler will care of allswitchstatements insrc/qt/guiutil.cpp:)
jonatack commented at 8:15 pm on January 10, 2021:True. Nevertheless, per element of least surprise, if we list the sites to update we may as well list all of them and save developer time.in src/qt/guiutil.cpp:770 in e711200759 outdated
763@@ -764,10 +764,10 @@ QString NetworkToQString(Network net) 764 assert(false); 765 } 766 767-QString ConnectionTypeToQString(ConnectionType conn_type) 768+QString ConnectionTypeToQString(ConnectionType conn_type, bool relay_txes) 769 { 770 switch (conn_type) { 771- case ConnectionType::INBOUND: return QObject::tr("Inbound"); 772+ case ConnectionType::INBOUND: return relay_txes ? QObject::tr("Inbound Full Relay") : QObject::tr("Inbound Block Relay");
hebasto commented at 7:49 pm on January 10, 2021:nit: Is a tooltip update required (see the screenshot)?
jonatack commented at 8:35 pm on January 10, 2021:Updated to Inbound Full/Block Relay
MarcoFalke commented at 7:10 am on January 28, 2021:SPV (light-wallet) peers are not “inbound block relay” peers
jonatack commented at 7:46 am on January 28, 2021:good point, fixing
in src/qt/rpcconsole.cpp:448 in e711200759 outdated
444@@ -444,7 +445,7 @@ void RPCExecutor::request(const QString &command, const WalletModel* wallet_mode 445 } 446 } 447 448-RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformStyle, QWidget *parent) : 449+RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle* _platformStyle, QWidget* parent) :
hebasto commented at 7:49 pm on January 10, 2021:nit: This change is unrelated to this PR :)
jonatack commented at 8:36 pm on January 10, 2021:removed (clang-format did it ;)in src/qt/rpcconsole.cpp:463 in e711200759 outdated
459@@ -459,11 +460,22 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty 460 461 ui->splitter->restoreState(settings.value("PeersTabSplitterSizes").toByteArray()); 462 463- QChar nonbreaking_hyphen(8209); 464+ const QChar nonbreaking_hyphen(8209);
hebasto commented at 7:51 pm on January 10, 2021:nit:
0 constexpr QChar nonbreaking_hyphen(8209);
jonatack commented at 8:36 pm on January 10, 2021:done, thankshebasto approvedhebasto commented at 7:51 pm on January 10, 2021: memberACK e711200759c4e772e029c5014863791d23597516, tested on Linux Mint 20.1 (Qt 5.12.8):
gui: return inbound {full, block} relay type in peer details 4f09615733f3153dc08fgui: improve markup handling of connection type tooltip
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
doc: update ConnectionType Doxygen documentation 79a2576af1jonatack force-pushed on Jan 10, 2021jonatack commented at 8:42 pm on January 10, 2021: contributorThanks @hebasto! Updated.
0diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp 1index 4fdc74c48..44addeeda 100644 2--- a/src/qt/rpcconsole.cpp 3+++ b/src/qt/rpcconsole.cpp 4@@ -445,7 +445,7 @@ void RPCExecutor::request(const QString &command, const WalletModel* wallet_mode 5 } 6 } 7 8-RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle* _platformStyle, QWidget* parent) : 9+RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformStyle, QWidget *parent) : 10 QWidget(parent), 11 m_node(node), 12 ui(new Ui::RPCConsole), 13@@ -460,9 +460,9 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle* _platformSty 14 15 ui->splitter->restoreState(settings.value("PeersTabSplitterSizes").toByteArray()); 16 17- const QChar nonbreaking_hyphen(8209); 18+ constexpr QChar nonbreaking_hyphen(8209); 19 const std::vector<QString> CONNECTION_TYPE_DOC{ 20- tr("Inbound: initiated by peer"), 21+ tr("Inbound Full/Block Relay: initiated by peer"), 22 tr("Outbound Full Relay: default"), 23 tr("Outbound Block Relay: does not relay transactions or addresses"), 24 tr("Outbound Manual: added using RPC %1 or %2/%3 configuration options")
hebasto approvedin src/qt/forms/debugwindow.ui:1082 in f3153dc08f outdated
1078@@ -1079,7 +1079,7 @@ 1079 <item row="1" column="0"> 1080 <widget class="QLabel" name="peerConnectionTypeLabel"> 1081 <property name="toolTip"> 1082- <string>The type of peer connection:<ul><li>Inbound Full/Block Relay: initiated by peer</li><li>Outbound Full Relay: default</li><li>Outbound Block Relay: does not relay transactions or addresses</li><li>Outbound Manual: added using RPC %1 or %2/%3 configuration options</li><li>Outbound Feeler: short-lived, for testing addresses</li><li>Outbound Address Fetch: short-lived, for soliciting addresses</li></ul></string> 1083+ <string>The type of peer connection: %1</string>
promag commented at 1:50 pm on January 11, 2021:f3153dc08fa16aa3bbca52f67833e5c9fbe8e095
Just move the whole tooltip? Why have a bit here?
hebasto commented at 2:05 pm on January 11, 2021:Why have a bit here?
In Qt Designer it makes obvious a tooltip presence.
jonatack commented at 2:23 pm on January 11, 2021:Just move the whole tooltip? Why have a bit here?
That’s what I started with in #163, see #163 (comment)
There have been a few iterations on this tooltip 😄
promag commented at 2:42 pm on January 11, 2021:I see, still not fond of the approach. Being a dynamic tooltip I think it’s fine to not be in the .ui file. If someone adds a tooltip in the designer then it will be obvious that it’s being replaced at runtime. Reading this code makes you check what is in the .ui, and you still have to check this code if you change what’s in the .ui. At the end anyone that wants to change this still has to check both places.
I think that a good rule of thumb is to leave all static/constant stuff for .ui.
Not saying that this is incorrect or that it doesn’t work, just my opinion. It’s fine if it’s merged as is.
jonatack commented at 2:53 pm on January 11, 2021:I don’t have a strong opinion and saw other tooltips with html markup completely in the .cpp file, but I also don’t have Qt or Qt Designer experience. I’m happy to keep it as-is or move it, either way.promag commented at 1:53 pm on January 11, 2021: contributorConcept ACK.jarolrod commented at 8:12 pm on January 15, 2021: memberACK 79a2576af1e499102943aa4e1d98994ee8a9c6b5, tested on macOS 11.1 with Qt 5.15.2
RandyMcMillan commented at 8:39 pm on January 26, 2021: contributorI don’t believe this much information should be displayed in a mouse over event. Attaching a click event to a UILabel (or a question mark image) would be a better approach (imo). The click event could trigger a modal view that gives a more detailed explanation of the information contained in the entire panel.jonatack commented at 8:49 pm on January 26, 2021: contributorThis pull doesn’t add the tooltip. It makes these follow-ups to #163:
- return relay type for inbound peers
- improve markup handling in the tooltip to facilitate translations
- update ConnectionType doxygen documentation
(Feel free to open a pull to propose your approach.)
laanwj approvedlaanwj commented at 7:05 pm on January 27, 2021: memberCode review ACK 79a2576af1e499102943aa4e1d98994ee8a9c6b5laanwj merged this on Jan 27, 2021laanwj closed this on Jan 27, 2021
jonatack deleted the branch on Jan 27, 2021jonatack commented at 7:54 pm on January 27, 2021: contributorThanks!sidhujag referenced this in commit 450c6b029e on Jan 28, 2021bitcoin-core locked this on Aug 16, 2022
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin-core/gui. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-23 02:20 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
