Differential D15140 Diff 41450 kdevplatform/shell/mainwindow_p.cpp

Changeset View

Standalone View

View Options

kdevplatform/shell/mainwindow_p.cpp

Show All 36 Lines
37 37​
38#include <util/pushvalue.h> 38​#include <util/pushvalue.h>
39 39​
40#include <interfaces/iplugin.h> 40​#include <interfaces/iplugin.h>
41 41​
42#include "core.h" 42​#include "core.h"
43#include "partdocument.h" 43​#include "partdocument.h"
44#include "partcontroller.h" 44​#include "partcontroller.h"
45​#include "plugincontroller.h"
45#include "uicontroller.h" 46​#include "uicontroller.h"
46#include "statusbar.h" 47​#include "statusbar.h"
47#include "mainwindow.h" 48​#include "mainwindow.h"
48#include "textdocument.h" 49​#include "textdocument.h"
49#include "sessioncontroller.h" 50​#include "sessioncontroller.h"
50#include "sourceformattercontroller.h" 51​#include "sourceformattercontroller.h"
51#include "debug.h" 52​#include "debug.h"
52#include "ktexteditorpluginintegration.h" 53​#include "ktexteditorpluginintegration.h"
53#include "colorschemechooser.h" 54​#include "colorschemechooser.h"
54 55​
55#include <language/duchain/duchainlock.h> 56​#include <language/duchain/duchainlock.h>
56#include <language/duchain/duchainutils.h> 57​#include <language/duchain/duchainutils.h>
57#include <language/duchain/topducontext.h> 58​#include <language/duchain/topducontext.h>
58#include <sublime/container.h> 59​#include <sublime/container.h>
59#include <sublime/holdupdates.h> 60​#include <sublime/holdupdates.h>
60 61​#include <qtcompat_p.h>
61 62​
62namespace KDevelop { 63​namespace KDevelop {
63 64​
64MainWindowPrivate::MainWindowPrivate(MainWindow *mainWindow) 65​MainWindowPrivate::MainWindowPrivate(MainWindow *mainWindow)
65 : QObject(mainWindow) 66​ : QObject(mainWindow)
66 , m_mainWindow(mainWindow) 67​ , m_mainWindow(mainWindow)
67 , m_statusBar(nullptr) 68​ , m_statusBar(nullptr)
68 , lastXMLGUIClientView(nullptr) 69​ , lastXMLGUIClientView(nullptr)
Show All 10 Lines
79 80​
80void MainWindowPrivate::setupStatusBar() 81​void MainWindowPrivate::setupStatusBar()
81{ 82​{
82 QWidget *location = m_mainWindow->statusBarLocation(); 83​ QWidget *location = m_mainWindow->statusBarLocation();
83 if (m_statusBar) 84​ if (m_statusBar)
84 location->layout()->addWidget(m_statusBar); 85​ location->layout()->addWidget(m_statusBar);
85} 86​}
86 87​
88​void sortMenuAlphabetically(QMenu* menu)
kossebauUnsubmitted
Done

-> sortMenuAlphabetically

kossebau: -> sortMenuAlphabetically
kossebauUnsubmitted
Not Done

Add also static, to keep the funtion symbol private to this compilation unit, which would be better behaviour.
IIUC, as we are in the KDevelop namespace here, not the anonymous one, so the symbol is exported from the compilation unit, and might result in clashes with similar util methods from other compilation units on creating the linkage unit (or whatever the term is).

kossebau: Add also `static`, to keep the funtion symbol private to this compilation unit, which would be…
89​{
90​ auto actions = menu->actions();
91​ for (auto action : qAsConst(actions)) {
92​ menu->removeAction(action);
93​ }
94​
95​ std::sort(actions.begin(), actions.end(), [](const QAction* left, const QAction* right) {
96​ const auto leftPlainText = left->text().remove(QLatin1Char('&'));
97​ const auto rightPlainText = right->text().remove(QLatin1Char('&'));
pinoUnsubmitted
Not Done

please use KLocalizedString::removeAcceleratorMarker() instead of removing & (see its API doc to understand why it is important)

pino: please use `KLocalizedString::removeAcceleratorMarker()` instead of removing `&` (see its API…
antonanikinAuthorUnsubmitted
Not Done

Don't know about this method earlier, thanks!

antonanikin: Don't know about this method earlier, thanks!
98​
99​ return (leftPlainText.localeAwareCompare(rightPlainText) < 0);
100​ });
101​
102​ for (auto action : qAsConst(actions)) {
103​ menu->addAction(action);
104​ }
105​}
106​
87void MainWindowPrivate::addPlugin( IPlugin *plugin ) 107​void MainWindowPrivate::addPlugin( IPlugin *plugin )
88{ 108​{
89 qCDebug(SHELL) << "add plugin" << plugin << plugin->componentName(); 109​ qCDebug(SHELL) << "add plugin" << plugin << plugin->componentName();
90 Q_ASSERT( plugin ); 110​ Q_ASSERT( plugin );
91 111​
92 //The direct plugin client can only be added to the first mainwindow 112​ //The direct plugin client can only be added to the first mainwindow
93 if(m_mainWindow == Core::self()->uiControllerInternal()->mainWindows()[0]) 113​ if(m_mainWindow == Core::self()->uiControllerInternal()->mainWindows()[0])
94 m_mainWindow->guiFactory()->addClient( plugin ); 114​ m_mainWindow->guiFactory()->addClient( plugin );
95 115​
96 Q_ASSERT(!m_pluginCustomClients.contains(plugin)); 116​ Q_ASSERT(!m_pluginCustomClients.contains(plugin));
97 117​
98 KXMLGUIClient* ownClient = plugin->createGUIForMainWindow(m_mainWindow); 118​ KXMLGUIClient* ownClient = plugin->createGUIForMainWindow(m_mainWindow);
99 if(ownClient) { 119​ if(ownClient) {
100 m_pluginCustomClients[plugin] = ownClient; 120​ m_pluginCustomClients[plugin] = ownClient;
101 connect(plugin, &IPlugin::destroyed, this, &MainWindowPrivate::pluginDestroyed); 121​ connect(plugin, &IPlugin::destroyed, this, &MainWindowPrivate::pluginDestroyed);
102 m_mainWindow->guiFactory()->addClient(ownClient); 122​ m_mainWindow->guiFactory()->addClient(ownClient);
103 } 123​ }
124​
125​ // Ensure alphabetical order in "Analzye Current * With" submenus
kossebauUnsubmitted
Done

Please add a comment in the code why this is done, like:

// Ensure alphabetical order in "Analzye Current * With" submenus
kossebau: Please add a comment in the code why this is done, like: ``` // Ensure alphabetical order in…
126​ const auto info = Core::self()->pluginController()->pluginInfo(plugin);
127​ if (info.category() == QStringLiteral("Analyzers")) {
kossebauUnsubmitted
Done

why static? that might be fragile, there is no promise in the kxmlgui API that submenu instances and their actions persist all the time, or?

kossebau: why static? that might be fragile, there is no promise in the kxmlgui API that submenu…
pinoUnsubmitted
Not Done

this sounds better as QLatin1String, since a real QString is not needed

pino: this sounds better as `QLatin1String`, since a real QString is not needed
128​ auto fileMenu = static_cast<QMenu*>(
kossebauUnsubmitted
Done

Analyzer plugins are not required to add an action to the main menu, so if there is no plugin which added an action to the submenu, this assert will wrongly fail, no?
So for what I understand no menu/action should be an expected condition and normally handled by skipping.

kossebau: Analyzer plugins are not required to add an action to the main menu, so if there is no plugin…
kossebauUnsubmitted
Done

From my testing it seems that the kxmlgui item for the submenu is always created, even when there is no content added. Had not found this documented though, but perhaps we can run with the QASSERT for now.
In any case though let's use official KXmlGui API to get the submenu :)

auto fileMenu = static_cast<QMenu*>(m_mainWindow->guiFactory()->container(QStringLiteral("analyze_file"), m_mainWindow));
        auto projectMenu = static_cast<QMenu*>(m_mainWindow->guiFactory()->container(QStringLiteral("analyze_project"), m_mainWindow));
kossebau: From my testing it seems that the kxmlgui item for the submenu is always created, even when…
129​ m_mainWindow->guiFactory()->container(QStringLiteral("analyze_file"), m_mainWindow));
130​ Q_ASSERT(fileMenu);
131​
kossebauUnsubmitted
Done

static?

kossebau: static?
132​ auto projectMenu = static_cast<QMenu*>(
kossebauUnsubmitted
Done

See above, no action could be expected.

kossebau: See above, no action could be expected.
133​ m_mainWindow->guiFactory()->container(QStringLiteral("analyze_project"), m_mainWindow));
134​ Q_ASSERT(projectMenu);
135​
136​ sortMenuAlphabetically(fileMenu);
137​ sortMenuAlphabetically(projectMenu);
138​ }
104} 139​}
105 140​
106void MainWindowPrivate::pluginDestroyed(QObject* pluginObj) 141​void MainWindowPrivate::pluginDestroyed(QObject* pluginObj)
107{ 142​{
108 IPlugin* plugin = static_cast<IPlugin*>(pluginObj); 143​ IPlugin* plugin = static_cast<IPlugin*>(pluginObj);
109 KXMLGUIClient* p = m_pluginCustomClients.take(plugin); 144​ KXMLGUIClient* p = m_pluginCustomClients.take(plugin);
110 m_mainWindow->guiFactory()->removeClient( p ); 145​ m_mainWindow->guiFactory()->removeClient( p );
111 delete p; 146​ delete p;
▲ Show 20 Lines • Show All 372 Lines • Show Last 20 Lines