broulik requested changes to this revision.
broulik added a comment.
This revision now requires changes to proceed.
Why not change the recentdocuments runner? We have various places where we
whitelist recentdocuments as a runner and if a user disabled it, this would not
be carried over without a confi update script.
Also, you might want to look at the old runner code for some of its behavior
(e.g. the subtext and mimetype handling) to keep it consistent.
INLINE COMMENTS
> recentlyused.cpp:93
> + }
> + match.setIconName(KIO::iconNameForUrl(url));
> + match.setRelevance(relevance);
I think plain mime type icon would be sufficient?
> recentlyused.cpp:95
> + match.setRelevance(relevance);
> + match.setData(url);
> + match.setText(name);
`setData` takes `QVariant`, so you can just store the `QUrl` in here, saves you
the string to URL dances below
> recentlyused.cpp:97
> + match.setText(name);
> + match.setSubtext(i18n("In %1",
> url.adjusted(QUrl::RemoveFilename).path()));
> +
We typically just show the folder path and use `~` for home, cf. existing runner
> recentlyused.cpp:109
> +
> + if (match.selectedAction() && match.selectedAction() ==
> action(s_openParentDirId)) {
> + KIO::highlightInFileManager({QUrl(url)});
the first part isn't needed, the comparison is enough
> recentlyused.cpp:125
> +
> + if (url.isLocalFile() && !QFileInfo(url.toLocalFile()).isDir()) {
> + actions << action(s_openParentDirId);
I think it's perfectly fine trying to show where a folder is inside another
folder
> recentlyused.cpp:135
> + QMimeData *result = new QMimeData();
> + result->setText(match.data().toString());
> + return result;
`setUrls`
> recentlyused.h:25
> +
> +#include <QIcon>
> +
Unused in the header
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D26111
To: meven, #plasma, ivan, ngraham, broulik
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
