Skip to content

Add "Direction" column to peers tab #289

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

Closed
wants to merge 4 commits into from
Closed

Add "Direction" column to peers tab #289

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
678b572
gui: add Direction column to peers tab
jonatack Apr 23, 2021
842f4e8
gui: improve connection type translations and translator comments
jonatack Apr 25, 2021
ad92e04
gui: use SPACE constant, QStringBuilder concat in rpcconsole.cpp
jonatack Apr 25, 2021
2bf3f5a
gui: add missing QLatin1String, QStringBuilder headers
jonatack Apr 25, 2021
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
Prev Previous commit
Next Next commit
gui: improve connection type translations and translator comments
  • Loading branch information
@jonatack
jonatack committed Apr 28, 2021
commit 842f4e834dfe5fd2786a5092f78ea28da1b36e4f
2 changes: 2 additions & 0 deletions src/qt/guiutil.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,10 +666,12 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction
{
QString prefix;
if (prepend_direction) {
//: Connection direction. Also used in CONNECTION_TYPE_DOC and PeerTableModel::data().
prefix = (conn_type == ConnectionType::INBOUND) ? QObject::tr("Inbound") : QObject::tr("Outbound") + " ";
}
switch (conn_type) {
case ConnectionType::INBOUND: return prefix;
//: The connection types (Full Relay, Block Relay, Manual, Feeler, Address Fetch) are also used in CONNECTION_TYPE_DOC.
case ConnectionType::OUTBOUND_FULL_RELAY: return prefix + QObject::tr("Full Relay");
case ConnectionType::BLOCK_RELAY: return prefix + QObject::tr("Block Relay");
case ConnectionType::MANUAL: return prefix + QObject::tr("Manual");
Expand Down
25 changes: 16 additions & 9 deletions src/qt/rpcconsole.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@
#include <QDateTime>
#include <QFont>
#include <QKeyEvent>
#include <QLatin1String>
#include <QMenu>
#include <QMessageBox>
#include <QScreen>
#include <QScrollBar>
#include <QSettings>
#include <QString>
#include <QStringBuilder>
Copy link

@Talkless Talkless Apr 25, 2021

Choose a reason for hiding this comment

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

Not sure if it helps only by including (without using operator %), unless whole bitcoin-qt would be built with QT_USE_QSTRINGBUILDER.

Copy link
Member Author

@jonatack jonatack Apr 25, 2021

Choose a reason for hiding this comment

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

Added because this file already uses %.

Copy link

@Talkless Talkless Apr 26, 2021

Choose a reason for hiding this comment

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

I see, QLatin1String and QStringBuilder included because where simply missing, not because of new usage. New usage would be nice though, but I don't want to be too-QString-nazi..

Copy link
Member Author

@jonatack jonatack Apr 28, 2021

Choose a reason for hiding this comment

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

Thanks for the nudge, @Talkless. Looked up https://wiki.qt.io/Using_QString_Effectively and updated. Please let me know what you think.

#include <QStringList>
#include <QTime>
#include <QTimer>
Expand All @@ -53,6 +55,9 @@ const int INITIAL_TRAFFIC_GRAPH_MINS = 30;
const QSize FONT_RANGE(4, 40);
const char fontSizeSettingsKey[] = "consoleFontSize";

const QLatin1String COLON_SPACE{": "};
Copy link

@Talkless Talkless Apr 25, 2021

Choose a reason for hiding this comment

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

Interesting idea. It actually could be constexpr.

Copy link
Member Author

@jonatack jonatack Apr 25, 2021

Choose a reason for hiding this comment

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

That was the first thing I tried but the compiler doesn't like it.

Copy link

@Talkless Talkless Apr 26, 2021

Choose a reason for hiding this comment

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

That's strange, QLatin1String's constructors are constexpr-marked since at least 5.3: https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qstring.h?h=5.3#n89

Sure that wasn't type or something? What kind of message was that? And what compiler?

Copy link
Member Author

@jonatack jonatack Apr 28, 2021

Choose a reason for hiding this comment

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

This seemed relevant: https://stackoverflow.com/questions/56201976/qt-vs-constexpr-string-literal

Clang 9 and GCC 10.2.1:

  CXX      qt/libbitcoinqt_a-rpcconsole.o
qt/rpcconsole.cpp:58:32: error: constexpr variable 'COLON_SPACE' must be initialized by a constant expression
static constexpr QLatin1String COLON_SPACE{": "};
                               ^~~~~~~~~~~~~~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:91:93: note: non-constexpr function 'strlen' cannot be used in a constant expression
    Q_DECL_CONSTEXPR inline explicit QLatin1String(const char *s) noexcept : m_size(s ? int(strlen(s)) : 0), m_data(s) {}
                                                                                            ^
qt/rpcconsole.cpp:58:32: note: in call to 'QLatin1String(&": "[0])'
static constexpr QLatin1String COLON_SPACE{": "};
                               ^
qt/rpcconsole.cpp:59:32: error: constexpr variable 'SPACE' must be initialized by a constant expression
static constexpr QLatin1String SPACE{" "};
                               ^~~~~~~~~~
/usr/include/x86_64-linux-gnu/qt5/QtCore/qstring.h:91:93: note: non-constexpr function 'strlen' cannot be used in a constant expression
    Q_DECL_CONSTEXPR inline explicit QLatin1String(const char *s) noexcept : m_size(s ? int(strlen(s)) : 0), m_data(s) {}
                                                                                            ^
qt/rpcconsole.cpp:59:32: note: in call to 'QLatin1String(&" "[0])'
static constexpr QLatin1String SPACE{" "};
                               ^
2 errors generated.

const QLatin1String SPACE{" "};

const struct {
const char *url;
const char *source;
Expand Down Expand Up @@ -463,16 +468,18 @@ RPCConsole::RPCConsole(interfaces::Node& node, const PlatformStyle *_platformSty

constexpr QChar nonbreaking_hyphen(8209);
const std::vector<QString> CONNECTION_TYPE_DOC{
tr("Inbound: initiated by peer"),
tr("Outbound Full Relay: default"),
tr("Outbound Block Relay: does not relay transactions or addresses"),
tr("Outbound Manual: added using RPC %1 or %2/%3 configuration options")
//: The connection direction (Inbound/Outbound) is also used in ConnectionTypeToQString() and PeerTableModel::data().
//: The connection types (Full Relay, Block Relay, Manual, Feeler, Address Fetch) are also used in ConnectionTypeToQString().
tr("Inbound") % COLON_SPACE % tr("initiated by peer"),
tr("Outbound") % SPACE % tr("Full Relay") % COLON_SPACE % tr("default"),
tr("Outbound") % SPACE % tr("Block Relay") % COLON_SPACE % tr("does not relay transactions or addresses"),
tr("Outbound") % SPACE % tr("Manual") % COLON_SPACE % tr("added using RPC %1 or %2/%3 configuration options")
.arg("addnode")
.arg(QString(nonbreaking_hyphen) + "addnode")
.arg(QString(nonbreaking_hyphen) + "connect"),
tr("Outbound Feeler: short-lived, for testing addresses"),
tr("Outbound Address Fetch: short-lived, for soliciting addresses")};
const QString list{"<ul><li>" + Join(CONNECTION_TYPE_DOC, QString("</li><li>")) + "</li></ul>"};
.arg(QString(nonbreaking_hyphen) % "addnode")
.arg(QString(nonbreaking_hyphen) % "connect"),
tr("Outbound") % SPACE % tr("Feeler") % COLON_SPACE % tr("short-lived, for testing addresses"),
tr("Outbound") % SPACE % tr("Address Fetch") % COLON_SPACE % tr("short-lived, for soliciting addresses")};
const QString list{"<ul><li>" % Join(CONNECTION_TYPE_DOC, QString("</li><li>")) % "</li></ul>"};
ui->peerConnectionTypeLabel->setToolTip(ui->peerConnectionTypeLabel->toolTip().arg(list));
const QString hb_list{"<ul><li>\""
+ ts.to + "\" – " + tr("we selected the peer for high bandwidth relay") + "</li><li>\""
Expand Down