[Differential] [Commented On] D1813: Fix selected name filter with multiple mimetypes

[elvisangelaccio (Elvis Angelaccio)]

elvisangelaccio added inline comments.

INLINE COMMENTS

> dfaure wrote in kdeplatformfiledialoghelper.cpp:78
> Can you explain and document here what this function does, i.e. input args and return value? It's a bit confusing.
> 
> "kde" is a mimetype name, e.g. application/zip right?
> "list" is a list of .... mimetype names?
> 
> I'm confused because the calling code is
> 
>   return kde2QtFilter(options()->nameFilters(), m_dialog->selectedNameFilter());
> 
> which looks like "Plain Text (*.txt)" type filter to me, but I guess it can also be "Plain Text (text/plain)" ? Or is that wrong ?
> It's strange because in the QFileDialog docu, name filters and mimetype filters are two different things...
> 
> Documenting examples here would be useful.
> 
> I see that the patch adds support for the (*.txt) case. Then the commit log is incomplete, because this isn't just about multiple mimetypes, but also about (non-mimetype-based) name filters, right?
> The commit log also says "for some reason" which sounds like the problem isn't fully understood, and says "when we're about to return empty" which sounds too-last-minute and doesn't match this version of the patch.
> Please revise the commit log and document the inputs/outputs of this method, then look at adding a unittest ;)
> Thanks!

Note that the "manually set" name filters seem currently broken (there is a `QEXPECT_FAIL` in `testSelectNameFilter()`), which is why I was only considering the "official" filters defined in the shared-mime-info database.

Anyway, I'll try to write an unit test :)

REPOSITORY
  rPLASMAINTEGRATION Integration for Qt applications in Plasma

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: elvisangelaccio, #plasma
Cc: dfaure, graesslin, mart, plasma-devel, #plasma, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160719/bcf54da4/attachment-0001.html>