Skip to content

Replaced invalidateFilter - #1100

Open
stefonarch wants to merge 2 commits into
masterfrom
silence_warnings
Open

Replaced invalidateFilter#1100
stefonarch wants to merge 2 commits into
masterfrom
silence_warnings

Conversation

@stefonarch

Copy link
Copy Markdown
Member

Fixes some of the warnings here #1099

The vfs-menu.c part is done with help from LLMs, so this needs a good review.

@isf63

isf63 commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

There was a cleaner way to silence the const warnings, also in multiple files: #1103

@stefonarch

stefonarch commented Apr 29, 2026

Copy link
Copy Markdown
Member Author

Removed the warnings already fixed by #1104.

@tsujan

tsujan commented Apr 29, 2026

Copy link
Copy Markdown
Member

It's good but needs version check (6.10). Adding version checks for 6.10 isn't a good idea; instead we should bump the min. required version of Qt to 6.10 everywhere and stop supporting very old distros in git and the next release. So, please wait for now …

@tsujan tsujan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops! There's a problem that I missed: change(s) should be done between beginFilterChange(); and endFilterChange();. So, this needs fixing.

Comment thread src/placesview.cpp
hiddenItemsRestored_ = true;
invalidateFilter();
beginFilterChange();
endFilterChange();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes should be done between beginFilterChange() and endFilterChange(), like this:

    if(!hiddenItemsRestored_ && !items.isEmpty()) {
        beginFilterChange();
        hidden_.clear();
        ....
        endFilterChange();

And also in the other cases.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done hopefully right.

Comment thread src/placesview.cpp
}
}

void PlacesProxyModel::setHidden(const QString& str, bool hide) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

beginFilterChange(); is missing here:

    beginFilterChange();
    if(hide) {

Comment thread src/placesview.cpp

void PlacesProxyModel::setHidden(const QString& str, bool hide) {
if(hide) {
beginFilterChange();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

beginFilterChange(); should be before if(hide) {.

@stefonarch stefonarch Apr 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

shouldn't do this work when tired as now...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should we rise Qt version in this PR too?

@stefonarch stefonarch changed the title Replaced invalidateFilter; split vars; Replaced invalidateFilter Apr 29, 2026

@tsujan tsujan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now it's OK, but please don't merge it until bumping of Qt version is decided and done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants