Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peer details: replace Direction with Connection Type #163

Merged
merged 4 commits into from
Jan 10, 2021
Merged

Peer details: replace Direction with Connection Type #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e2beab
gui: create GUIUtil::ConnectionTypeToQString utility function
jonatack Dec 24, 2020
2c19ba2
gui: replace Direction with Connection Type in peer details
jonatack Dec 24, 2020
c95fe6e
gui: improve connection type tooltip
hebasto Dec 25, 2020
06ba9b3
rpc: move getpeerinfo connection_type help to correct place
jonatack Dec 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/qt/forms/debugwindow.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,14 +1077,17 @@
</widget>
</item>
<item row="1" column="0">
<widget class="QLabel" name="label_23">
<widget class="QLabel" name="peerConnectionTypeLabel">
<property name="toolTip">
<string>The type of peer connection:&lt;ul&gt;&lt;li&gt;Inbound: initiated by peer&lt;/li&gt;&lt;li&gt;Outbound Full Relay: default&lt;/li&gt;&lt;li&gt;Outbound Block Relay: does not relay transactions or addresses&lt;/li&gt;&lt;li&gt;Outbound Manual: added using RPC %1 or %2/%3 configuration options&lt;/li&gt;&lt;li&gt;Outbound Feeler: short-lived, for testing addresses&lt;/li&gt;&lt;li&gt;Outbound Address Fetch: short-lived, for soliciting addresses&lt;/li&gt;&lt;/ul&gt;</string>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translation nit: How are translators supposed to translate :&lt;ul&gt;&lt;li&gt;? It might be better to say "Can be one of %s" and then use Join(translated_strings, ", ") or simply "Refer to the getpeerinfo RPC help for details on the connection types"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hebasto, if we replace the escaped markup with placeholders (as done here for the nonbreaking hyphen), is that still QT Designer-friendly? Can do it in #179.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

translation nit: How are translators supposed to translate :&lt;ul&gt;&lt;li&gt;?

There was a discussion in private IRC with @jonatack:

<hebasto>
14:24:52 but I expect the translators' burden :)
14:25:39 hope that translators are aware of escaping :)

I'm not an expert in translation but I believe that :&lt;ul&gt;&lt;li&gt; is not a subject to translate as %1 or %2.

It might be better to say "Can be one of %s" and then use Join(translated_strings, ", ") or simply "Refer to the getpeerinfo RPC help for details on the connection types"?

Either of the suggested variants LGTM.

if we replace the escaped markup with placeholders (as done here for the nonbreaking hyphen), is that still QT Designer-friendly?

I think it is. But, tbh, I'd prefer more succinct @MarcoFalke's suggestions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Helpfulness and readability for users were the reasons for the iterations above and it seems best to not regress on those improvements with referring GUI users to the command line or dropping the readable list layout. I'll propose replacing the escaped markup with placeholders.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not a single placeholder for the whole unordered list? We still have a (shorter) tooltip in the form file, and make translators' life easier :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in #180

</property>
<property name="text">
<string>Direction</string>
<string>Connection Type</string>
</property>
</widget>
</item>
<item row="1" column="1">
<widget class="QLabel" name="peerDirection">
<widget class="QLabel" name="peerConnectionType">
<property name="cursor">
<cursorShape>IBeamCursor</cursorShape>
</property>
Expand Down
13 changes: 13 additions & 0 deletions src/qt/guiutil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,19 @@ QString NetworkToQString(Network net)
assert(false);
}

QString ConnectionTypeToQString(ConnectionType conn_type)
{
switch (conn_type) {
case ConnectionType::INBOUND: return QObject::tr("Inbound");
case ConnectionType::OUTBOUND_FULL_RELAY: return QObject::tr("Outbound Full Relay");
case ConnectionType::BLOCK_RELAY: return QObject::tr("Outbound Block Relay");
case ConnectionType::MANUAL: return QObject::tr("Outbound Manual");
case ConnectionType::FEELER: return QObject::tr("Outbound Feeler");
case ConnectionType::ADDR_FETCH: return QObject::tr("Outbound Address Fetch");
} // no default case, so the compiler can warn about missing cases
assert(false);
}

QString formatDurationStr(int secs)
{
QStringList strList;
Expand Down
6 changes: 5 additions & 1 deletion src/qt/guiutil.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,18 @@

#include <amount.h>
#include <fs.h>
#include <net.h>
#include <netaddress.h>

#include <QEvent>
#include <QHeaderView>
#include <QItemDelegate>
#include <QLabel>
#include <QMessageBox>
#include <QObject>
#include <QProgressBar>
#include <QString>
#include <QTableView>
#include <QLabel>

class QValidatedLineEdit;
class SendCoinsRecipient;
Expand Down Expand Up @@ -228,6 +229,9 @@ namespace GUIUtil
/** Convert enum Network to QString */
QString NetworkToQString(Network net);

/** Convert enum ConnectionType to QString */
QString ConnectionTypeToQString(ConnectionType conn_type);

/** Convert seconds into a QString with days, hours, mins, secs */
QString formatDurationStr(int secs);

Expand Down
3 changes: 2 additions & 1 deletion src/qt/rpcconsole.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,7 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty
ui->dataDir->setToolTip(ui->dataDir->toolTip().arg(QString(nonbreaking_hyphen) + "datadir"));
ui->blocksDir->setToolTip(ui->blocksDir->toolTip().arg(QString(nonbreaking_hyphen) + "blocksdir"));
ui->openDebugLogfileButton->setToolTip(ui->openDebugLogfileButton->toolTip().arg(PACKAGE_NAME));
ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg("addnode").arg(QString(nonbreaking_hyphen) + "addnode").arg(QString(nonbreaking_hyphen) + "connect"));

if (platformStyle->getImagesOnButtons()) {
ui->openDebugLogfileButton->setIcon(platformStyle->SingleColorIcon(":/icons/export"));
Expand Down Expand Up @@ -1111,7 +1112,7 @@ void RPCConsole::updateDetailWidget()
ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset));
ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
ui->peerDirection->setText(stats->nodeStats.fInbound ? tr("Inbound") : tr("Outbound"));
ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type));
ui->peerNetwork->setText(GUIUtil::NetworkToQString(stats->nodeStats.m_network));
if (stats->nodeStats.m_permissionFlags == PF_NONE) {
ui->peerPermissions->setText(tr("N/A"));
Expand Down
6 changes: 3 additions & 3 deletions src/rpc/net.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ static RPCHelpMan getpeerinfo()
{RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"},
{RPCResult::Type::BOOL, "bip152_hb_to", "Whether we selected peer as (compact blocks) high-bandwidth peer"},
{RPCResult::Type::BOOL, "bip152_hb_from", "Whether peer selected us as (compact blocks) high-bandwidth peer"},
{RPCResult::Type::STR, "connection_type", "Type of connection: \n" + Join(CONNECTION_TYPE_DOC, ",\n") + ".\n"
"Please note this output is unlikely to be stable in upcoming releases as we iterate to\n"
"best capture connection behaviors."},
{RPCResult::Type::NUM, "startingheight", "The starting height (block) of the peer"},
{RPCResult::Type::NUM, "synced_headers", "The last header we have in common with this peer"},
{RPCResult::Type::NUM, "synced_blocks", "The last block we have in common with this peer"},
Expand All @@ -156,6 +153,9 @@ static RPCHelpMan getpeerinfo()
"Only known message types can appear as keys in the object and all bytes received\n"
"of unknown message types are listed under '"+NET_MESSAGE_COMMAND_OTHER+"'."}
}},
{RPCResult::Type::STR, "connection_type", "Type of connection: \n" + Join(CONNECTION_TYPE_DOC, ",\n") + ".\n"
"Please note this output is unlikely to be stable in upcoming releases as we iterate to\n"
"best capture connection behaviors."},
}},
}},
},
Expand Down