Replace QRegExp with QRegularExpression#606
Closed
jarolrod wants to merge 1 commit intobitcoin-core:masterfrom
Closed
Replace QRegExp with QRegularExpression#606jarolrod wants to merge 1 commit intobitcoin-core:masterfrom
jarolrod wants to merge 1 commit intobitcoin-core:masterfrom
Conversation
Contributor
Author
hebasto
reviewed
May 25, 2022
Member
hebasto
left a comment
hebasto
left a comment
There was a problem hiding this comment.
Concept ACK.
Suggesting to apply clang-format-diff.py and use QStringLiteral macro:
--- a/src/qt/utilitydialog.cpp
+++ b/src/qt/utilitydialog.cpp
@@ -23,6 +23,7 @@
#include <QLabel>
#include <QMainWindow>
#include <QRegularExpression>
+#include <QString>
#include <QTextCursor>
#include <QTextTable>
#include <QVBoxLayout>
@@ -44,8 +45,8 @@ HelpMessageDialog::HelpMessageDialog(QWidget *parent, bool about) :
/// HTML-format the license message from the core
QString licenseInfoHTML = QString::fromStdString(LicenseInfo());
// Make URLs clickable
- QRegularExpression uri("<(.*)>", QRegularExpression::InvertedGreedinessOption);
- licenseInfoHTML.replace(uri, "<a href=\"\\1\">\\1</a>");
+ QRegularExpression uri(QStringLiteral("<(.*)>"), QRegularExpression::InvertedGreedinessOption);
+ licenseInfoHTML.replace(uri, QStringLiteral("<a href=\"\\1\">\\1</a>"));
// Replace newlines with HTML breaks
licenseInfoHTML.replace("\n", "<br>");
Still reviewing guiutil.cpp changes.
Member
|
Echoing: #585 (comment) |
Member
|
It seems the code is broken both in master and in this branch. It does not do what is supposed to do Line 313 in dd19c1a I mean, it fails to extract first suffix from filter pattern "Description (*.foo *.bar ...)". However, all of the callers in our codebase do not use patterns with multiple suffixes. |
Member
Ok, an inversion of greediness will help: --- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -311,7 +311,7 @@ QString getSaveFileName(QWidget *parent, const QString &caption, const QString &
QString result = QDir::toNativeSeparators(QFileDialog::getSaveFileName(parent, caption, myDir, filter, &selectedFilter));
/* Extract first suffix from filter pattern "Description (*.foo)" or "Description (*.foo *.bar ...) */
- QRegularExpression filter_re(".* \\(\\*\\.(.*)[ \\)]");
+ QRegularExpression filter_re(QStringLiteral(".* \\(\\*\\.(.*)[ \\)]"), QRegularExpression::InvertedGreedinessOption);
QString selectedSuffix;
QRegularExpressionMatch m = filter_re.match(selectedFilter);
if(m.hasMatch()) |
Contributor
Agree. |
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Member
|
Labeled "Up for grabs"... |
Member
|
Closing in favor of #620. |
laanwj
added a commit
to bitcoin/bitcoin
that referenced
this pull request
Jun 22, 2022
67364eb test, qt: Add tests for `GUIUtil::extractFirstSuffixFromFilter` (w0xlt) ace9af5 qt: Replace `QRegExp` with `QRegularExpression` (w0xlt) c378535 qt: Add a function that extracts the suffix from a filter (w0xlt) Pull request description: Picking up bitcoin-core/gui#606 (labeled "Up for grabs") and applying bitcoin-core/gui#606 (review) and bitcoin-core/gui#606 (comment). Replaces occurrences of `QRegExp` usage with `QRegularExpression` as part of the roadmap for Qt6 integration. Fixes bitcoin-core/gui#578 ACKs for top commit: laanwj: Code review and lightly tested ACK 67364eb hebasto: ACK 67364eb Tree-SHA512: 4a17d83e557bc635cbd1a15776856e9edb7162b23a369ccbd2ac59c68b8a1ea663baaa7d5ad98e419dc03b91ef3315c768eeadc01c0b29162de109493161e814
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Picking up #585
Replaces occurrences of QRegExp usage with QRegularExpression as part of the roadmap for Qt6 integration.
fixes #578