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
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
