D15140: Fix random order in "Analyze Current File/Project With" menus

[Friedrich W. H. Kossebau]

kossebau added a comment.


  First round of code feedback. Still need to check whether kxmlgui is fine with us twisting the generated QMenu behind its back, or if that will confuse things.

INLINE COMMENTS

> mainwindow_p.cpp:88
>  
> +void sortMenu(QMenu* menu)
> +{

-> sortMenuAlphabetically

> mainwindow_p.cpp:125
> +
> +    const auto info = Core::self()->pluginController()->pluginInfo(plugin);
> +    if (info.category() == QStringLiteral("Analyzers")) {

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

  // Ensure alphabetical order in "Analzye Current * With" submenus

> mainwindow_p.cpp:127
> +    if (info.category() == QStringLiteral("Analyzers")) {
> +        static auto fileMenuAction = m_mainWindow->findChild<QAction*>(QStringLiteral("analyze_file"));
> +        Q_ASSERT(fileMenuAction);

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?

> mainwindow_p.cpp:128
> +        static auto fileMenuAction = m_mainWindow->findChild<QAction*>(QStringLiteral("analyze_file"));
> +        Q_ASSERT(fileMenuAction);
> +        Q_ASSERT(fileMenuAction->menu());

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.

> mainwindow_p.cpp:131
> +
> +        static auto projectMenuAction = m_mainWindow->findChild<QAction*>(QStringLiteral("analyze_project"));
> +        Q_ASSERT(projectMenuAction);

static?

> mainwindow_p.cpp:132
> +        static auto projectMenuAction = m_mainWindow->findChild<QAction*>(QStringLiteral("analyze_project"));
> +        Q_ASSERT(projectMenuAction);
> +        Q_ASSERT(projectMenuAction->menu());

See above, no action could be expected.

REPOSITORY
  R32 KDevelop

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

To: antonanikin, #kdevelop
Cc: kossebau, kdevelop-devel, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20180910/79d41405/attachment-0001.html>