Differential D10363 Diff 36488 src/filewidgets/knewfilemenu.cpp

Changeset View

Standalone View

View Options

src/filewidgets/knewfilemenu.cpp

Show First 20 Lines • Show All 633 Lines • ▼ Show 20 Line(s)
634 634​
635void KNewFileMenuPrivate::fillMenu() 635​void KNewFileMenuPrivate::fillMenu()
636{ 636​{
637 QMenu *menu = q->menu(); 637​ QMenu *menu = q->menu();
638 menu->clear(); 638​ menu->clear();
639 m_menuDev->menu()->clear(); 639​ m_menuDev->menu()->clear();
640 m_newDirAction = nullptr; 640​ m_newDirAction = nullptr;
641 641​
642 QSet<QString> seenTexts; 642​ QSet<QString> seenTexts;
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…
643​ QString lastTemplatePath;
643 // these shall be put at special positions 644​ // these shall be put at special positions
644 QAction *linkURL = nullptr; 645​ QAction *linkURL = nullptr;
645 QAction *linkApp = nullptr; 646​ QAction *linkApp = nullptr;
646 QAction *linkPath = nullptr; 647​ QAction *linkPath = nullptr;
647 648​
648 KNewFileMenuSingleton *s = kNewMenuGlobals(); 649​ KNewFileMenuSingleton *s = kNewMenuGlobals();
649 int i = 1; 650​ int i = 1;
650 KNewFileMenuSingleton::EntryList::iterator templ = s->templatesList->begin(); 651​ KNewFileMenuSingleton::EntryList::iterator templ = s->templatesList->begin();
Show All 36 Lines671 if (templatePath.endsWith(QLatin1String("emptydir"))) {
687 } 688​ }
688 689​
689 menu->addAction(act); 690​ menu->addAction(act);
690 691​
691 QAction *sep = new QAction(q); 692​ QAction *sep = new QAction(q);
692 sep->setSeparator(true); 693​ sep->setSeparator(true);
693 menu->addAction(sep); 694​ menu->addAction(sep);
694 } else { 695​ } else {
695 696​ if (lastTemplatePath.startsWith(QDir::homePath()) && !templatePath.startsWith(QDir::homePath())) {
697​ QAction *sep = new QAction(q);
698​ sep->setSeparator(true);
699​ menu->addAction(sep);
700​ }
696 if (!m_supportedMimeTypes.isEmpty()) { 701​ if (!m_supportedMimeTypes.isEmpty()) {
697 bool keep = false; 702​ bool keep = false;
698 703​
699 // We need to do mimetype filtering, for real files. 704​ // We need to do mimetype filtering, for real files.
700 const bool createSymlink = entry.templatePath == QLatin1String("__CREATE_SYMLINK__"); 705​ const bool createSymlink = entry.templatePath == QLatin1String("__CREATE_SYMLINK__");
701 if (createSymlink) { 706​ if (createSymlink) {
702 keep = true; 707​ keep = true;
703 } else if (!KDesktopFile::isDesktopFile(entry.templatePath)) { 708​ } else if (!KDesktopFile::isDesktopFile(entry.templatePath)) {
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Line(s)748 } else if (KDesktopFile::isDesktopFile(templatePath)) {
747 } else { 752​ } else {
748 menu->addAction(act); 753​ menu->addAction(act);
749 } 754​ }
750 } else { 755​ } else {
751 menu->addAction(act); 756​ menu->addAction(act);
752 } 757​ }
753 } 758​ }
754 } 759​ }
760​ lastTemplatePath = entry.templatePath;
755 } else { // Separate system from personal templates 761​ } else { // Separate system from personal templates
756 Q_ASSERT(entry.entryType != 0); 762​ Q_ASSERT(entry.entryType != 0);
757 763​
758 QAction *sep = new QAction(q); 764​ QAction *sep = new QAction(q);
759 sep->setSeparator(true); 765​ sep->setSeparator(true);
760 menu->addAction(sep); 766​ menu->addAction(sep);
761 } 767​ }
762 } 768​ }
▲ Show 20 Lines • Show All 119 Lines • ▼ Show 20 Line(s)887{
882 _k_slotCreateDirectory(true); 888​ _k_slotCreateDirectory(true);
883} 889​}
884 890​
885struct EntryWithName { 891​struct EntryWithName {
886 QString key; 892​ QString key;
887 KNewFileMenuSingleton::Entry entry; 893​ KNewFileMenuSingleton::Entry entry;
888}; 894​};
889 895​
890void KNewFileMenuPrivate::_k_slotFillTemplates() 896​void KNewFileMenuPrivate::_k_slotFillTemplates()
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.
891{ 897​{
892 KNewFileMenuSingleton *s = kNewMenuGlobals(); 898​ KNewFileMenuSingleton *s = kNewMenuGlobals();
893 //qDebug(); 899​ //qDebug();
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 900​
895 const QStringList qrcTemplates = { QStringLiteral(":/kio5/newfile-templates") }; 901​ const QStringList qrcTemplates = { QStringLiteral(":/kio5/newfile-templates") };
896 const QStringList installedTemplates = QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("templates"), QStandardPaths::LocateDirectory); 902​ QStringList installedTemplates = { QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("templates"), QStandardPaths::LocateDirectory) };
dfaureUnsubmitted
Done

typo: receive

dfaure: typo: receive
903​ // Qt does not provide an easy way to receive the xdg dir for templates so we have to find it on our own
904​ #ifdef Q_OS_UNIX
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​ QString xdgUserDirs = QStandardPaths::locate(QStandardPaths::ConfigLocation, QStringLiteral("user-dirs.dirs"), QStandardPaths::LocateFile);
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​ QFile xdgUserDirsFile(xdgUserDirs);
907​ if (xdgUserDirsFile.open(QIODevice::ReadOnly | QIODevice::Text)) {
908​ QTextStream in(&xdgUserDirsFile);
909​ while (!in.atEnd()) {
910​ QString line = in.readLine();
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​ if (line.startsWith("XDG_TEMPLATES_DIR=")) {
912​ QString xdgTemplates = line.mid(19, line.size()-20);
913​ xdgTemplates.replace(QString("$HOME"), QDir::homePath());
914​ QDir xdgTemplatesDir(xdgTemplates);
915​ if (xdgTemplatesDir.exists()) {
dfaureUnsubmitted
Done

not needed, the destructor closes.

dfaure: not needed, the destructor closes.
916​ installedTemplates << xdgTemplates;
917​ }
918​ break;
919​ }
920​ }
921​ }
922​ #endif
923​
897 const QStringList templates = qrcTemplates + installedTemplates; 924​ const QStringList templates = qrcTemplates + installedTemplates;
898 925​
899 // Ensure any changes in the templates dir will call this 926​ // Ensure any changes in the templates dir will call this
900 if (! s->dirWatch) { 927​ if (! s->dirWatch) {
901 s->dirWatch = new KDirWatch; 928​ s->dirWatch = new KDirWatch;
902 for (const QString &dir : installedTemplates) { 929​ for (const QString &dir : installedTemplates) {
903 s->dirWatch->addDir(dir); 930​ s->dirWatch->addDir(dir);
904 } 931​ }
Show All 36 Lines962 if (file[0] != '.') {
941 // and TextFile before others because it's the most used one. 968​ // and TextFile before others because it's the most used one.
942 // This also sorts by user-visible name. 969​ // This also sorts by user-visible name.
943 // The rest of the re-ordering is done in fillMenu. 970​ // The rest of the re-ordering is done in fillMenu.
944 const KDesktopFile config(file); 971​ const KDesktopFile config(file);
945 QString url = config.desktopGroup().readEntry("URL"); 972​ QString url = config.desktopGroup().readEntry("URL");
946 QString key = config.desktopGroup().readEntry("Name"); 973​ QString key = config.desktopGroup().readEntry("Name");
947 if (file.endsWith(QLatin1String("Directory.desktop"))) { 974​ if (file.endsWith(QLatin1String("Directory.desktop"))) {
948 key.prepend('0'); 975​ key.prepend('0');
949 } else if (file.endsWith(QLatin1String("TextFile.desktop"))) { 976​ } else if (file.startsWith(QDir::homePath())) {
950 key.prepend('1'); 977​ key.prepend('1');
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…
951 } else { 978​ } else if (file.endsWith(QLatin1String("TextFile.desktop"))) {
952 key.prepend('2'); 979​ key.prepend('2');
980​ } else {
981​ key.prepend('3');
953 } 982​ }
954 EntryWithName en = { key, e }; 983​ EntryWithName en = { key, e };
955 if (ulist.contains(url)) { 984​ if (ulist.contains(url)) {
956 ulist.remove(url); 985​ ulist.remove(url);
957 } 986​ }
958 ulist.insert(url, en); 987​ ulist.insert(url, en);
959 } 988​ }
960 } 989​ }
▲ Show 20 Lines • Show All 309 Lines • Show Last 20 Lines