D21249: Test current filter before setting a new one

David Faure noreply at phabricator.kde.org
Sun May 19 12:31:28 BST 2019


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfilewidget.cpp:2463
> +        if (rx.exactMatch(filename)) {
> +            if (bUpdate && p != QLatin1String("*")) {   // never match the catch-all filter
> +                filterWidget->setCurrentFilter(filter);

Is the bUpdate bool necessary?

Without it, we'd call setCurrentFilter(currentFilter()) which should be a no-op, right?

Alternatively, the method could return a QString, and the (second) caller could call setCurrentFilter.

I just don't like a method that's sometimes a getter and sometimes a setter (basically).

> kfilewidget.cpp:2495
>              QString filename = urlStr.mid(urlStr.lastIndexOf(QLatin1Char('/')) + 1);     // only filename
> +            if (matchFilter(filename, filterWidget->currentFilter() + QLatin1Char('|'), false))
> +                return;

The added '|' isn't needed, is it?

str.left(-1) returns the whole string.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D21249

To: jglogowski, #frameworks, ngraham, dfaure
Cc: elvisangelaccio, ngraham, michaelweghorn, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190519/06d91eef/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list