D5446: Implement support for selected mime type filters

David Faure noreply at phabricator.kde.org
Sat Apr 15 08:02:01 UTC 2017


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

INLINE COMMENTS

> kfiledialog_unittest.cpp:84
> +
> +        QMimeType headerMime = QMimeDatabase().mimeTypeForName(QStringLiteral("text/x-chdr"));
> +        QMimeType jsonMime = QMimeDatabase().mimeTypeForName(QStringLiteral("application/json"));

Why use QMimeDatabase, just to then use name()? You could just put "text/x-chdr" in the data row, no?

> kfiledialog_unittest.cpp:116
> +        QEXPECT_FAIL("", "QFileDialog in Qt < 5.9 does not report the selected mime type.", Continue);
> +        QCOMPARE(QString(), targetMimeTypeFilter);
> +#endif

Surely you mean QCOMPARE(dialog.selectedMimeTypeFilter(), QString())

> kdeplatformfiledialoghelper.cpp:181
> +{
> +    Q_ASSERT(m_fileWidget->filterWidget()->isMimeFilter());
> +    return m_fileWidget->filterWidget()->currentFilter();

Are you sure this ASSERT is wanted? See next comment.

> qfiledialogtest.cpp:116
> +#if QT_VERSION >= QT_VERSION_CHECK(5, 9, 0)
> +        qDebug() << "selected mime type filter" << dialog.selectedMimeTypeFilter();
> +#endif

this code looks ok to me, but it will hit the above assert when the -mimeTypeFilter option is absent, no?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: elvisangelaccio, #plasma, dfaure
Cc: plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170415/2ab345b7/attachment.html>


More information about the Plasma-devel mailing list