Rework cancellation logic for LayerFeaturesModel searches#4526
Conversation
Coverage Report for CI Build 27276990546Warning No base build found for commit Coverage: 57.877%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
📦 Build Artifacts Ready
|
📦 Build Artifacts Ready
|
Withalion
left a comment
There was a problem hiding this comment.
let's rebase before merging
| beginResetModel(); | ||
| mFeatures.clear(); | ||
| for ( const auto &f : features ) | ||
| QFutureWatcher<SearchResultData> *watcher = static_cast< QFutureWatcher<SearchResultData> *>( sender() ); |
There was a problem hiding this comment.
Should be dynamic_cast instead
There was a problem hiding this comment.
The sender here is guaranteed to be the future watcher.
I'd argue that actually crashing would be a better choice in case future me tries to call onFutureFinished() by other means!
There was a problem hiding this comment.
Alright, I don't really have a strong argument here. It was also pointed out by clang-tidy
There was a problem hiding this comment.
If type safety is the issue, I'd suggest a0a46c0
abda380 to
a0a46c0
Compare
|
I switched back to using the |
📦 Build Artifacts Ready
|
Rework of the cancellation logic in
LayerFeaturesModelfeature requests.Main changes:
QgsFeedbackandQFutureWatcherinstances.QgsFeedback::cancel()and let the provider stop the iteration.