D21249: Test current filter before setting a new one

David Faure noreply at phabricator.kde.org
Tue May 21 08:17:07 BST 2019


dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Almost there. Thanks for *your* patience :-)

INLINE COMMENTS

> kfilewidgettest.cpp:88
> +        // setUrl runs with blocked signals, so use setUrls
> +        // auto-select ODT filter via filename
> +        fw.locationEdit()->setUrls(QStringList(QStringLiteral("test.odt")));

... use setUrls *to* auto-select ODT filter?

("to" is missing)

> kfilewidget.cpp:2456
>  
> +QString KFileWidgetPrivate::findMatchingFilter(const QString &filter, const QString &filename)
> +{

This method could (and should) be marked as const, now.

> kfilewidget.cpp:2492
>              QString filename = urlStr.mid(urlStr.lastIndexOf(QLatin1Char('/')) + 1);     // only filename
> +            // don't test "*" against a match to honor the user's selection, as it's never auto-selected
> +            if (!findMatchingFilter(filterWidget->currentFilter(), filename).isEmpty()) {

I don't really understand what this comment is doing here, it seems unrelated to the next line of code. Is it just a longer version of the comment on line 2499?

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/20190521/c57e1e73/attachment.html>


More information about the Kde-frameworks-devel mailing list