EditorManager: Avoid changing the editor in between when closing editors

Do not call setCurrentEditor(nullptr) since this sends a
currentEditorChanged signal which claims that no editor is active, which
can lead to unnecessary updates.
In the specific case, the git branch view was updating with the project
directory as a new repository, and then, after the actual new current
editor was set updated again with the repository of the editor.
This can be needlessly expensive if the previous and new editor are in
the same repository, but the project repository is something else, like
when having the Qt Creator super-repository open, or when editing files
unrelated to the project.

Change-Id: I515c3c40a86bfa2713ff13460ddfda974bc08c59
Reviewed-by: David Schulz <david.schulz@qt.io>

src/plugins/coreplugin/editormanager/editormanager.cpp

@@ -1650,62 +1650,91 @@ bool EditorManagerPrivate::closeEditors(const QList<IEditor*> &editors, CloseFla
     if (acceptedEditors.isEmpty())
         return false;
 
-    QList<EditorView*> closedViews;
+    // save editor states
-    EditorView *focusView = nullptr;
+    for (IEditor *editor : qAsConst(acceptedEditors)) {
-
+        if (!editor->document()->filePath().isEmpty() && !editor->document()->isTemporary()) {
-    // remove the editors
-    foreach (IEditor *editor, acceptedEditors) {
-        emit m_instance->editorAboutToClose(editor);
-        if (!editor->document()->filePath().isEmpty()
-                && !editor->document()->isTemporary()) {
             QByteArray state = editor->saveState();
             if (!state.isEmpty())
                 d->m_editorStates.insert(editor->document()->filePath().toString(), QVariant(state));
         }
+    }
 
+    EditorView *focusView = nullptr;
+
+    // Remove accepted editors from document model/manager and context list,
+    // and sort them per view, so we can remove them from views in an orderly
+    // manner.
+    QMultiHash<EditorView *, IEditor *> editorsPerView;
+    for (IEditor *editor : qAsConst(acceptedEditors)) {
+        emit m_instance->editorAboutToClose(editor);
         removeEditor(editor, flag != CloseFlag::Suspend);
         if (EditorView *view = viewForEditor(editor)) {
-            if (QApplication::focusWidget() && QApplication::focusWidget() == editor->widget()->focusWidget())
+            editorsPerView.insert(view, editor);
+            if (QApplication::focusWidget()
+                && QApplication::focusWidget() == editor->widget()->focusWidget()) {
                 focusView = view;
-            if (editor == view->currentEditor())
-                closedViews += view;
-            if (d->m_currentEditor == editor) {
-                // avoid having a current editor without view
-                setCurrentView(view);
-                setCurrentEditor(nullptr);
             }
-            view->removeEditor(editor);
         }
     }
+    QTC_CHECK(!focusView || focusView == currentView);
 
-    // TODO doesn't work as expected with multiple areas in main window and some other cases
+    // Go through views, remove the editors from them.
-    // instead each view should have its own file history and handle solely themselves
+    // Sort such that views for which the current editor is closed come last,
-    // which editor is shown if their current editor closes
+    // and if the global current view is one of them, that comes very last.
-    EditorView *forceViewToShowEditor = nullptr;
+    // When handling the last view in the list we handle the case where all
-    if (!closedViews.isEmpty() && EditorManager::visibleEditors().isEmpty()) {
+    // visible editors are closed, and we need to e.g. revive an invisible or
-        if (closedViews.contains(currentView))
+    // a suspended editor
-            forceViewToShowEditor = currentView;
+    QList<EditorView *> views = editorsPerView.keys();
-        else
+    Utils::sort(views, [editorsPerView, currentView](EditorView *a, EditorView *b) {
-            forceViewToShowEditor = closedViews.first();
+        if (a == b)
-    }
+            return false;
-    foreach (EditorView *view, closedViews) {
+        const bool aHasCurrent = editorsPerView.values(a).contains(a->currentEditor());
-        IEditor *newCurrent = view->currentEditor();
+        const bool bHasCurrent = editorsPerView.values(b).contains(b->currentEditor());
-        if (!newCurrent && forceViewToShowEditor == view)
+        const bool aHasGlobalCurrent = (a == currentView && aHasCurrent);
-            newCurrent = pickUnusedEditor();
+        const bool bHasGlobalCurrent = (b == currentView && bHasCurrent);
-        if (newCurrent) {
+        if (bHasGlobalCurrent && !aHasGlobalCurrent)
-            activateEditor(view, newCurrent, EditorManager::DoNotChangeCurrentEditor);
+            return true;
-        } else if (forceViewToShowEditor == view) {
+        if (bHasCurrent && !aHasCurrent)
-            DocumentModel::Entry *entry = DocumentModelPrivate::firstSuspendedEntry();
+            return true;
-            if (entry) {
+        return false;
-                activateEditorForEntry(view, entry, EditorManager::DoNotChangeCurrentEditor);
+    });
-            } else { // no "suspended" ones, so any entry left should have a document
+    for (EditorView *view : qAsConst(views)) {
-                const QList<DocumentModel::Entry *> documents = DocumentModel::entries();
+        QList<IEditor *> editors = editorsPerView.values(view);
-                if (!documents.isEmpty()) {
+        // handle current editor in view last
-                    if (IDocument *document = documents.last()->document) {
+        IEditor *viewCurrentEditor = view->currentEditor();
-                        activateEditorForDocument(view, document, EditorManager::DoNotChangeCurrentEditor);
+        if (editors.contains(viewCurrentEditor) && editors.last() != viewCurrentEditor) {
+            editors.removeAll(viewCurrentEditor);
+            editors.append(viewCurrentEditor);
+        }
+        for (IEditor *editor : qAsConst(editors)) {
+            if (editor == viewCurrentEditor && view == views.last()) {
+                // Avoid removing the globally current editor from its view,
+                // set a new current editor before.
+                const EditorManager::OpenEditorFlags flags = view != currentView
+                                                                 ? EditorManager::DoNotChangeCurrentEditor
+                                                                 : EditorManager::NoFlags;
+                const QList<IEditor *> viewEditors = view->editors();
+                IEditor *newCurrent = viewEditors.size() > 1 ? viewEditors.at(viewEditors.size() - 2)
+                                                             : nullptr;
+                if (!newCurrent)
+                    newCurrent = pickUnusedEditor();
+                if (newCurrent) {
+                    activateEditor(view, newCurrent, flags);
+                } else {
+                    DocumentModel::Entry *entry = DocumentModelPrivate::firstSuspendedEntry();
+                    if (entry) {
+                        activateEditorForEntry(view, entry, flags);
+                    } else { // no "suspended" ones, so any entry left should have a document
+                        const QList<DocumentModel::Entry *> documents = DocumentModel::entries();
+                        if (!documents.isEmpty()) {
+                            if (IDocument *document = documents.last()->document) {
+                                activateEditorForDocument(view, document, flags);
+                            }
+                        }
                     }
                 }
             }
+            view->removeEditor(editor);
         }
     }