Differential D10363 Diff 26753 src/filewidgets/knewfilemenu.cpp

Changeset View

Standalone View

View Options

src/filewidgets/knewfilemenu.cpp

Context not available.
639 m_newDirAction = nullptr; 639​ m_newDirAction = nullptr;
640 640​
641 QSet<QString> seenTexts; 641​ QSet<QString> seenTexts;
642​ QString lastSeenText;
dfaureUnsubmitted
Done

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

dfaure: That's not a (user-visible) text, that's a path, please rename the variable to lastTemplatePath…
642 // these shall be put at special positions 643​ // these shall be put at special positions
643 QAction *linkURL = nullptr; 644​ QAction *linkURL = nullptr;
644 QAction *linkApp = nullptr; 645​ QAction *linkApp = nullptr;
Context not available.
691 sep->setSeparator(true); 692​ sep->setSeparator(true);
692 menu->addAction(sep); 693​ menu->addAction(sep);
693 } else { 694​ } else {
694 695​ if (lastSeenText.startsWith(QDir::homePath()) && !templatePath.startsWith(QDir::homePath())) {
696​ QAction *sep = new QAction(q);
697​ sep->setSeparator(true);
698​ menu->addAction(sep);
699​ }
695 if (!m_supportedMimeTypes.isEmpty()) { 700​ if (!m_supportedMimeTypes.isEmpty()) {
696 bool keep = false; 701​ bool keep = false;
697 702​
Context not available.
751 } 756​ }
752 } 757​ }
753 } 758​ }
759​ lastSeenText = entry.templatePath;
754 } else { // Separate system from personal templates 760​ } else { // Separate system from personal templates
755 Q_ASSERT(entry.entryType != 0); 761​ Q_ASSERT(entry.entryType != 0);
756 762​
dfaureUnsubmitted
Done

All your new code should be in #ifdef Q_OS_UNIX, it is not applicable on e.g. Windows.

dfaure: All your new code should be in #ifdef Q_OS_UNIX, it is not applicable on e.g. Windows.
Context not available.
892 //qDebug(); 898​ //qDebug();
893 899​
dfaureUnsubmitted
Done

remove the exists() check, open will fail if it doesn't exist, anyway, so we might as well save one syscall.

dfaure: remove the exists() check, open will fail if it doesn't exist, anyway, so we might as well save…
894 const QStringList qrcTemplates = { QStringLiteral(":/kio5/newfile-templates") }; 900​ const QStringList qrcTemplates = { QStringLiteral(":/kio5/newfile-templates") };
895 const QStringList installedTemplates = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("templates"), QStandardPaths::LocateDirectory); 901​ QStringList installedTemplates = { QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("templates"), QStandardPaths::LocateDirectory) };
902​ // Qt does not provide an easy way to recieve the xdg dir for templates so we have to find it on our own
dfaureUnsubmitted
Done

typo: receive

dfaure: typo: receive
903​ #ifdef Q_OS_UNIX
904​ QString xdgUserDirs = QStandardPaths::locate(QStandardPaths::ConfigLocation, QStringLiteral("user-dirs.dirs"), QStandardPaths::LocateFile);
dfaureUnsubmitted
Done

the 2nd arg isn't necessary, just do line.mid(19)

dfaure: the 2nd arg isn't necessary, just do line.mid(19)
mmustacAuthorUnsubmitted
Done

It is because we need to get rid of the " before and at the end of the path.

mmustac: It is because we need to get rid of the " before and at the end of the path.
dfaureUnsubmitted
Done

Ah I see, OK.

dfaure: Ah I see, OK.
905​ QFile xdgUserDirsFile(xdgUserDirs);
dfaureUnsubmitted
Done

QStringLiteral("$HOME/")

No need to use QStandardPaths to get the home dir (that's... historical), use QDir::homePath() directly.

dfaure: QStringLiteral("$HOME/") No need to use QStandardPaths to get the home dir (that's...
906​ if (xdgUserDirsFile.open(QIODevice::ReadOnly | QIODevice::Text)) {
907​ QTextStream in(&xdgUserDirsFile);
908​ while (!in.atEnd()) {
909​ QString line = in.readLine();
910​ if (line.startsWith("XDG_TEMPLATES_DIR=")) {
dfaureUnsubmitted
Done

wait, what? Interesting that this compiles, it's not the Qt way to clear a string (that would be .clear()). But just kill the line (and therefore the else), it's unnecessary, the variable is going out of scope anyway.

dfaure: wait, what? Interesting that this compiles, it's not the Qt way to clear a string (that would…
911​ QString xdgTemplates = line.mid(19, line.size()-20);
912​ xdgTemplates.replace(QString("$HOME"), QDir::homePath());
913​ QDir xdgTemplatesDir(xdgTemplates);
914​ if (xdgTemplatesDir.exists()) {
915​ installedTemplates << xdgTemplates;
dfaureUnsubmitted
Done

not needed, the destructor closes.

dfaure: not needed, the destructor closes.
916​ }
917​ break;
918​ }
919​ }
920​ }
921​ #endif
922​
896 const QStringList templates = qrcTemplates + installedTemplates; 923​ const QStringList templates = qrcTemplates + installedTemplates;
897 924​
898 // Ensure any changes in the templates dir will call this 925​ // Ensure any changes in the templates dir will call this
Context not available.
946 if (file.endsWith(QLatin1String("Directory.desktop"))) { 973​ if (file.endsWith(QLatin1String("Directory.desktop"))) {
947 key.prepend('0'); 974​ key.prepend('0');
948 } else if (file.endsWith(QLatin1String("TextFile.desktop"))) { 975​ } else if (file.endsWith(QLatin1String("TextFile.desktop"))) {
976​ key.prepend('2');
977​ } else if (file.startsWith(QDir::homePath())) {
dfaureUnsubmitted
Done

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?

dfaure: can you swap the conditions so that this code still is ordered 0, 1, 2, 3? Makes it more…
949 key.prepend('1'); 978​ key.prepend('1');
950 } else { 979​ } else {
951 key.prepend('2'); 980​ key.prepend('3');
952 } 981​ }
953 EntryWithName en = { key, e }; 982​ EntryWithName en = { key, e };
954 if (ulist.contains(url)) { 983​ if (ulist.contains(url)) {
Context not available.