Simplify Open with... menu handling

In the light of lambdas we can get rid of some complexity.

Change-Id: Id28b52c42d7c34526c93785723387b6c44c0cd52
Reviewed-by: Daniel Teske <daniel.teske@theqtcompany.com>
Reviewed-by: Nikita Baryshnikov <nib952051@gmail.com>
Reviewed-by: David Schulz <david.schulz@theqtcompany.com>

src/plugins/coreplugin/documentmanager.cpp

@@ -118,14 +118,6 @@ static bool saveModifiedFilesHelper(const QList<IDocument *> &documents,
 
 namespace Internal {
 
-struct OpenWithEntry
-{
-    OpenWithEntry() : editorFactory(0), externalEditor(0) {}
-    IEditorFactory *editorFactory;
-    IExternalEditor *externalEditor;
-    QString fileName;
-};
-
 struct FileStateItem
 {
     QDateTime modified;
@@ -210,8 +202,6 @@ DocumentManagerPrivate::DocumentManagerPrivate() :
 } // namespace Internal
 } // namespace Core
 
-Q_DECLARE_METATYPE(Core::Internal::OpenWithEntry)
-
 namespace Core {
 
 using namespace Internal;
@@ -1385,6 +1375,44 @@ void DocumentManager::notifyFilesChangedInternally(const QStringList &files)
     emit m_instance->filesChangedInternally(files);
 }
 
+static void openEditorWith(const QString &fileName, Core::Id editorId)
+{
+    // close any open editors that have this file open
+    // remember the views to open new editors in there
+    QList<EditorView *> views;
+    QList<IEditor *> editorsOpenForFile
+            = DocumentModel::editorsForFilePath(fileName);
+    foreach (IEditor *openEditor, editorsOpenForFile) {
+        EditorView *view = EditorManagerPrivate::viewForEditor(openEditor);
+        if (view && view->currentEditor() == openEditor) // visible
+            views.append(view);
+    }
+    if (!EditorManager::closeEditors(editorsOpenForFile)) // don't open if cancel was pressed
+        return;
+
+    if (views.isEmpty()) {
+        EditorManager::openEditor(fileName, editorId);
+    } else {
+        if (EditorView *currentView = EditorManagerPrivate::currentEditorView()) {
+            if (views.removeOne(currentView))
+                views.prepend(currentView); // open editor in current view first
+        }
+        EditorManager::OpenEditorFlags flags;
+        foreach (EditorView *view, views) {
+            IEditor *editor = EditorManagerPrivate::openEditor(view, fileName, editorId, flags);
+            // Do not change the current editor after opening the first one. That
+            // * prevents multiple updates of focus etc which are not necessary
+            // * lets us control which editor is made current by putting the current editor view
+            //   to the front (if that was in the list in the first place)
+            flags |= EditorManager::DoNotChangeCurrentEditor;
+            // do not try to open more editors if this one failed, or editor type does not
+            // support duplication anyhow
+            if (!editor || !editor->duplicateSupported())
+                break;
+        }
+    }
+}
+
 void DocumentManager::populateOpenWithMenu(QMenu *menu, const QString &fileName)
 {
     typedef QList<IEditorFactory*> EditorFactoryList;
@@ -1403,75 +1431,32 @@ void DocumentManager::populateOpenWithMenu(QMenu *menu, const QString &fileName)
         if (anyMatches) {
             // Add all suitable editors
             foreach (IEditorFactory *editorFactory, factories) {
+                Core::Id editorId = editorFactory->id();
                 // Add action to open with this very editor factory
                 QString const actionTitle = editorFactory->displayName();
-                QAction * const action = menu->addAction(actionTitle);
+                QAction *action = menu->addAction(actionTitle);
-                OpenWithEntry entry;
+                // Below we need QueuedConnection because otherwise, if a qrc file
-                entry.editorFactory = editorFactory;
+                // is inside of a qrc file itself, and the qrc editor opens the Open with menu,
-                entry.fileName = fileName;
+                // crashes happen, because the editor instance is deleted by openEditorWith
-                action->setData(qVariantFromValue(entry));
+                // while the menu is still being processed.
+                connect(action, &QAction::triggered, EditorManager::instance(),
+                        [fileName, editorId]() {
+                            openEditorWith(fileName, editorId);
+                        }, Qt::QueuedConnection);
             }
             // Add all suitable external editors
             foreach (IExternalEditor *externalEditor, externalEditors) {
-                QAction * const action = menu->addAction(externalEditor->displayName());
+                QAction *action = menu->addAction(externalEditor->displayName());
-                OpenWithEntry entry;
+                Core::Id editorId = externalEditor->id();
-                entry.externalEditor = externalEditor;
+                connect(action, &QAction::triggered, [fileName, editorId]() {
-                entry.fileName = fileName;
+                    EditorManager::openExternalEditor(fileName, editorId);
-                action->setData(qVariantFromValue(entry));
+                });
             }
         }
     }
     menu->setEnabled(anyMatches);
 }
 
-void DocumentManager::executeOpenWithMenuAction(QAction *action)
-{
-    QTC_ASSERT(action, return);
-    const QVariant data = action->data();
-    OpenWithEntry entry = qvariant_cast<OpenWithEntry>(data);
-    if (entry.editorFactory) {
-        // close any open editors that have this file open
-        // remember the views to open new editors in there
-        QList<EditorView *> views;
-        QList<IEditor *> editorsOpenForFile
-                = DocumentModel::editorsForFilePath(entry.fileName);
-        foreach (IEditor *openEditor, editorsOpenForFile) {
-            EditorView *view = EditorManagerPrivate::viewForEditor(openEditor);
-            if (view && view->currentEditor() == openEditor) // visible
-                views.append(view);
-        }
-        if (!EditorManager::closeEditors(editorsOpenForFile)) // don't open if cancel was pressed
-            return;
-
-        if (views.isEmpty()) {
-            EditorManager::openEditor(entry.fileName, entry.editorFactory->id());
-        } else {
-            if (EditorView *currentView = EditorManagerPrivate::currentEditorView()) {
-                if (views.removeOne(currentView))
-                    views.prepend(currentView); // open editor in current view first
-            }
-            EditorManager::OpenEditorFlags flags;
-            foreach (EditorView *view, views) {
-                IEditor *editor =
-                        EditorManagerPrivate::openEditor(view, entry.fileName,
-                                                         entry.editorFactory->id(), flags);
-                // Do not change the current editor after opening the first one. That
-                // * prevents multiple updates of focus etc which are not necessary
-                // * lets us control which editor is made current by putting the current editor view
-                //   to the front (if that was in the list in the first place
-                flags |= EditorManager::DoNotChangeCurrentEditor;
-                // do not try to open more editors if this one failed, or editor type does not
-                // support duplication anyhow
-                if (!editor || !editor->duplicateSupported())
-                    break;
-            }
-        }
-        return;
-    }
-    if (entry.externalEditor)
-        EditorManager::openExternalEditor(entry.fileName, entry.externalEditor->id());
-}
-
 bool DocumentManager::eventFilter(QObject *obj, QEvent *e)
 {
     if (obj == qApp && e->type() == QEvent::ApplicationActivate) {

src/plugins/coreplugin/documentmanager.h

@@ -143,9 +143,6 @@ public:
        lead to any editors to reload or any other editor manager actions. */
     static void notifyFilesChangedInternally(const QStringList &files);
 
-public slots:
-    static void executeOpenWithMenuAction(QAction *action);
-
 signals:
     /* Used to notify e.g. the code model to update the given files. Does *not*
        lead to any editors to reload or any other editor manager actions. */

src/plugins/coreplugin/editormanager/editormanager.cpp

@@ -2085,8 +2085,6 @@ void EditorManager::addNativeDirAndOpenWithActions(QMenu *contextMenu, DocumentM
     contextMenu->addAction(d->m_openTerminalAction);
     contextMenu->addAction(d->m_findInDirectoryAction);
     QMenu *openWith = contextMenu->addMenu(tr("Open With"));
-    connect(openWith, SIGNAL(triggered(QAction*)),
-            DocumentManager::instance(), SLOT(executeOpenWithMenuAction(QAction*)));
     openWith->setEnabled(enabled);
     if (enabled)
         DocumentManager::populateOpenWithMenu(openWith, entry->fileName().toString());

src/plugins/projectexplorer/foldernavigationwidget.cpp

@@ -403,7 +403,6 @@ void FolderNavigationWidget::contextMenuEvent(QContextMenuEvent *ev)
         TextEditor::FindInFiles::findOnFileSystem(m_fileSystemModel->filePath(current));
         return;
     }
-    Core::DocumentManager::executeOpenWithMenuAction(action);
 }
 
 void FolderNavigationWidget::setHiddenFilesFilter(bool filter)

src/plugins/projectexplorer/projectexplorer.cpp

@@ -726,9 +726,6 @@ bool ProjectExplorerPlugin::initialize(const QStringList &arguments, QString *er
     dd->m_openWithMenu = openWith->menu();
     dd->m_openWithMenu->setTitle(tr("Open With"));
 
-    connect(dd->m_openWithMenu, &QMenu::triggered,
-            DocumentManager::instance(), &DocumentManager::executeOpenWithMenuAction);
-
     //
     // Separators
     //

src/plugins/resourceeditor/resourceeditorplugin.cpp

@@ -190,8 +190,6 @@ bool ResourceEditorPlugin::initialize(const QStringList &arguments, QString *err
     folderContextMenu->menu()->insertMenu(
                 folderContextMenu->insertLocation(ProjectExplorer::Constants::G_FOLDER_FILES),
                 m_openWithMenu);
-    connect(m_openWithMenu, &QMenu::triggered,
-            Core::DocumentManager::instance(), &Core::DocumentManager::executeOpenWithMenuAction);
 
     m_copyPath = new Utils::ParameterAction(QString(), tr("Copy path \"%1\""), Utils::ParameterAction::AlwaysEnabled, this);
     command = Core::ActionManager::registerAction(m_copyPath, Constants::C_COPY_PATH, projectTreeContext);

src/plugins/resourceeditor/resourceeditorw.cpp

@@ -103,14 +103,6 @@ ResourceEditorW::ResourceEditorW(const Core::Context &context,
     m_renameAction = m_contextMenu->addAction(tr("Rename File..."), this, SLOT(renameCurrentFile()));
     m_copyFileNameAction = m_contextMenu->addAction(tr("Copy Resource Path to Clipboard"), this, SLOT(copyCurrentResourcePath()));
 
-    // Below we need QueuedConnection because otherwise, if this qrc file
-    // is inside of the qrc file, crashes happen when using "Open With" on it.
-    // (That is because this editor instance is deleted in executeOpenWithMenuAction
-    // in that case.)
-    connect(m_openWithMenu, SIGNAL(triggered(QAction*)),
-            Core::DocumentManager::instance(), SLOT(executeOpenWithMenuAction(QAction*)),
-            Qt::QueuedConnection);
-
     connect(m_resourceEditor, SIGNAL(dirtyChanged(bool)), m_resourceDocument, SLOT(dirtyChanged(bool)));
     connect(m_resourceEditor, SIGNAL(undoStackChanged(bool,bool)),
             this, SLOT(onUndoStackChanged(bool,bool)));