dfaure added inline comments.
INLINE COMMENTS
> kdeplatformfiledialoghelper.cpp:78
> /*
> * Map a KDE filter string into a Qt one.
> */
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!
> kdeplatformfiledialoghelper.cpp:86
> + const int pos = filter.indexOf(kde);
> + if (pos > 0 && filter.length() >= kde.length() + pos) {
> + const QChar prev = filter[pos - 1];
(pre-existing issue) the ">=" should actually be ">" [and I would swap it
around and make it < for readability]
otherwise when pos + kde.length() == filter.length() then the "succ" line below
goes out of bounds.
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
