D10363: [KIO] Add support for XDG_TEMPLATES_DIR in KNewFileMenu

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Hello, sorry for the delay in my review, I failed to react to the notification and then it scrolled out of view.... :/

INLINE COMMENTS

> knewfilemenu.cpp:642
>      QSet<QString> seenTexts;
> +    QString lastSeenText;
>      // these shall be put at special positions

That's not a (user-visible) text, that's a path, please rename the variable to lastTemplatePath or something like that.

> knewfilemenu.cpp:902
> +    QStringList installedTemplates = { QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("templates"), QStandardPaths::LocateDirectory) };
> +    // Qt does not provide an easy way to recieve the xdg dir for templates so we have to find it on our own
> +    #ifdef Q_OS_UNIX

typo: receive

> knewfilemenu.cpp:977
> +                key.prepend('2');
> +            } else if (file.startsWith(QDir::homePath())) {
>                  key.prepend('1');

can you swap the conditions so that this code still is ordered 0, 1, 2, 3? Makes it more readable (since it will match the resulting menu order)

But actually, shouldn't we still have New Folder and New Text File at the top before everything else?

> mmustac wrote in knewfilemenu.cpp:904
> It is because we need to get rid of the " before and at the end of the path.

Ah I see, OK.

REPOSITORY
  R241 KIO

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

To: mmustac, #frameworks, dfaure
Cc: broulik, ngraham, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180428/a3acfdec/attachment.html>