Replace QRegExp with QRegularExpression by prusnak · Pull Request #585 · bitcoin-core/gui

prusnak · 2022-04-15T18:20:30Z

Fixes #578

Only made sure the code builds, not sure how to test.

hebasto · 2022-04-15T18:25:47Z

Concept ACK.

As a preparation to Qt 6, I think this PR will benefit by incorporating of bitcoin/bitcoin@f6adcad and bitcoin/bitcoin@b649103 in any reasonable way.

prusnak · 2022-04-15T18:31:52Z

As a preparation to Qt 6, I think this PR will benefit by incorporating of bitcoin/bitcoin@f6adcad and bitcoin/bitcoin@b649103 in any reasonable way.

bitcoin/bitcoin@f6adcad is already part of c6b6106
the latter commit I just cherry-picked to this PR

prusnak · 2022-04-15T22:29:38Z

It seems the CI is failing because it does not recognize filterRegularExpression and indeed it was introduced in Qt 5.12 while we still do support Qt 5.11.

I updated the second commit of the PR to use QT_VERSION check - see 54980a7

hebasto · 2022-04-21T18:00:42Z

Does it make sense to extract the "Extract first suffix from filter pattern..." functionality into a testable utility function?

hebasto · 2022-04-23T17:08:06Z

not sure how to test.

Here is a branch with tests for the AddressBookPage class -- https://github.com/hebasto/gui/commits/220423-test-ab

And test fails for the 54980a7 commit.

UPDATE: mind looking at #591, #592 and #593?

DrahtBot · 2022-04-24T03:51:16Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#593 (Getting ready to Qt 6 (8/n). Use QRegularExpression in AddressBookSortFilterProxyModel class by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

1506913 qt, test: Add tests for `tableView` in `AddressBookPage` dialog (Hennadii Stepanov) edae3ab qt: No need to force Qt::QueuedConnection for NotifyAddressBookChanged (Hennadii Stepanov) Pull request description: This PR is a prerequisite for more thorough testing of filtering in the `AddressBookPage` class in context of #578 and #585. Required for #592. ACKs for top commit: promag: Code review ACK 1506913. Tree-SHA512: 86986d47606cbd54d813436c7afb21894e2200b6d3042a7aa0b5e84821c765bd68b14ad38a445069891ab33f2d7bcd4933b8373e14e9afb0c91f1a6ddf4da740

…ssBookPage` dialog 1506913 qt, test: Add tests for `tableView` in `AddressBookPage` dialog (Hennadii Stepanov) edae3ab qt: No need to force Qt::QueuedConnection for NotifyAddressBookChanged (Hennadii Stepanov) Pull request description: This PR is a prerequisite for more thorough testing of filtering in the `AddressBookPage` class in context of bitcoin-core/gui#578 and bitcoin-core/gui#585. Required for bitcoin-core/gui#592. ACKs for top commit: promag: Code review ACK 1506913. Tree-SHA512: 86986d47606cbd54d813436c7afb21894e2200b6d3042a7aa0b5e84821c765bd68b14ad38a445069891ab33f2d7bcd4933b8373e14e9afb0c91f1a6ddf4da740

hebasto · 2022-05-23T15:09:11Z

src/qt/utilitydialog.cpp

        // Make URLs clickable
-        QRegExp uri("<(.*)>", Qt::CaseSensitive, QRegExp::RegExp2);
-        uri.setMinimal(true); // use non-greedy matching
+        QRegularExpression uri("<(.*)>", QRegularExpression::InvertedGreedinessOption); // use non-greedy matching


A comment seems unneeded here because QRegularExpression::InvertedGreedinessOption is pretty self-explained.

Addressed in db71143

jarolrod · 2022-05-24T00:45:46Z

src/qt/addressbookpage.cpp


        auto address = model->index(row, AddressTableModel::Address, parent);

+    #if (QT_VERSION >= QT_VERSION_CHECK(5, 12, 0))


slight preference for flipping this around where we're checking if less than qt6, i think this makes the code cleaner and inline with how we've done some other qt6 preparation changes, but not a blocker

Current implementation allows to test without Qt 6.

…`AddressBookSortFilterProxyModel` class e280087 qt: Use `QRegularExpression` in `AddressBookSortFilterProxyModel` class (Hennadii Stepanov) 5c5d8f2 qt, test: Add tests for searching in `AddressBookPage` dialog (Hennadii Stepanov) Pull request description: This is a step in [migration](bitcoin/bitcoin#24798) to Qt 6. Related: - #578 - #585 No behavior change. To ensure this, tests have been added. ACKs for top commit: hebasto: > tACK [e280087](e280087) on Ubuntu 21.10 Qt 5.15.2 promag: Tested ACK e280087 with Qt6 on macOS 12 M1. w0xlt: tACK e280087 on Ubuntu 21.10 Qt 5.15.2 jarolrod: Tested ACK e280087 on M1 mac, x86 mac, x86 Linux with Qt5 and separately with Qt6 Tree-SHA512: 664baacc1504deb2f7fa651ea4a44f3942f5c9058befe4d2ce292beed032d4b1697710cfd10c0909602d8a4a6eeb680414e4a1f56d2038478c1ae2f34965d74f

DrahtBot · 2022-05-24T09:23:12Z

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}

prusnak · 2022-05-24T09:55:18Z

I won't be able to work on this PR in the following days/weeks. Feel free to push into my branch (maintainers are allowed) or take over the PR completely if needed.

jarolrod · 2022-05-24T23:28:38Z

Picked up in #606

…rExpression` in `AddressBookSortFilterProxyModel` class e280087 qt: Use `QRegularExpression` in `AddressBookSortFilterProxyModel` class (Hennadii Stepanov) 5c5d8f2 qt, test: Add tests for searching in `AddressBookPage` dialog (Hennadii Stepanov) Pull request description: This is a step in [migration](bitcoin#24798) to Qt 6. Related: - bitcoin-core/gui#578 - bitcoin-core/gui#585 No behavior change. To ensure this, tests have been added. ACKs for top commit: hebasto: > tACK [e280087](bitcoin-core/gui@e280087) on Ubuntu 21.10 Qt 5.15.2 promag: Tested ACK e280087 with Qt6 on macOS 12 M1. w0xlt: tACK bitcoin-core/gui@e280087 on Ubuntu 21.10 Qt 5.15.2 jarolrod: Tested ACK bitcoin-core/gui@e280087 on M1 mac, x86 mac, x86 Linux with Qt5 and separately with Qt6 Tree-SHA512: 664baacc1504deb2f7fa651ea4a44f3942f5c9058befe4d2ce292beed032d4b1697710cfd10c0909602d8a4a6eeb680414e4a1f56d2038478c1ae2f34965d74f

prusnak mentioned this pull request Apr 15, 2022

Replace QRegExp with QRegularExpression #578

Closed

hebasto changed the title ~~qt: replace QRegExp with QRegularExpression~~ Replace QRegExp with QRegularExpression Apr 15, 2022

hebasto added Refactoring Qt 6 Transition to Qt 6 labels Apr 15, 2022

prusnak force-pushed the qregexp-obsolete branch from fb834e2 to 54980a7 Compare April 15, 2022 22:28

This was referenced Apr 23, 2022

test: Add tests for tableView in AddressBookPage dialog #591

Merged

Getting ready to Qt 6 (8/n). Use QRegularExpression in AddressBookSortFilterProxyModel class #593

Merged

DrahtBot mentioned this pull request Apr 24, 2022

Drop wildcard support for address/label search in AddressBookPage #592

Closed

hebasto reviewed May 23, 2022

View reviewed changes

prusnak and others added 2 commits May 23, 2022 18:32

qt: replace QRegExp with QRegularExpression

db71143

qt: Use QRegularExpression in AddressBookSortFilterProxyModel class

7b896dc

prusnak force-pushed the qregexp-obsolete branch from 54980a7 to 7b896dc Compare May 23, 2022 16:33

jarolrod reviewed May 24, 2022

View reviewed changes

DrahtBot added the Needs rebase label May 24, 2022

hebasto added the Up for grabs label May 24, 2022

jarolrod mentioned this pull request May 24, 2022

Replace QRegExp with QRegularExpression #606

Closed

jarolrod closed this May 24, 2022

hebasto removed the Up for grabs label May 25, 2022

prusnak deleted the qregexp-obsolete branch May 25, 2022 08:02

bitcoin-core locked and limited conversation to collaborators May 25, 2023


		auto address = model->index(row, AddressTableModel::Address, parent);

		#if (QT_VERSION >= QT_VERSION_CHECK(5, 12, 0))

Conversation

prusnak commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Apr 15, 2022

Uh oh!

prusnak commented Apr 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prusnak commented Apr 15, 2022

Uh oh!

hebasto commented Apr 21, 2022

Uh oh!

hebasto commented Apr 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Apr 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto May 23, 2022

Choose a reason for hiding this comment

Uh oh!

prusnak May 23, 2022

Choose a reason for hiding this comment

Uh oh!

jarolrod May 24, 2022

Choose a reason for hiding this comment

Uh oh!

hebasto May 24, 2022

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented May 24, 2022

Uh oh!

prusnak commented May 24, 2022

Uh oh!

jarolrod commented May 24, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

prusnak commented Apr 15, 2022 •

edited

Loading

prusnak commented Apr 15, 2022 •

edited

Loading

hebasto commented Apr 23, 2022 •

edited

Loading

DrahtBot commented Apr 24, 2022 •

edited

Loading