VCS: Simplify submit editor accept/close flow

* Replace message box with an error on the output pane.
* Separate logic for accept and close.

Change-Id: Ib9fa201a1a67ee195086f7a22bc6678a9642452a
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@qt.io>
Reviewed-by: André Hartmann <aha_1980@gmx.de>

11 files changed, 60 insertions, 88 deletions

diff --git a/src/plugins/bazaar/bazaarplugin.cpp b/src/plugins/bazaar/bazaarplugin.cpp
index 4f417ddc5e..33d13c631f 100644
--- a/src/plugins/bazaar/bazaarplugin.cpp
+++ b/src/plugins/bazaar/bazaarplugin.cpp
@@ -184,7 +184,7 @@ public:
     // String -> repository, StringList -> files
     void changed(const QVariant &);
     void updateActions(VcsBase::VcsBasePluginPrivate::ActionState) final;
-    bool submitEditorAboutToClose() final;
+    bool activateCommit() final;
 
     // File menu action slots
     void addCurrentFile();
@@ -784,7 +784,7 @@ void BazaarPluginPrivate::uncommit()
         m_client.synchronousUncommit(state.topLevel(), dialog.revision(), dialog.extraOptions());
 }
 
-bool BazaarPluginPrivate::submitEditorAboutToClose()
+bool BazaarPluginPrivate::activateCommit()
 {
     auto commitEditor = qobject_cast<CommitEditor *>(submitEditor());
     QTC_ASSERT(commitEditor, return true);
diff --git a/src/plugins/clearcase/clearcaseplugin.cpp b/src/plugins/clearcase/clearcaseplugin.cpp
index 6be0bc3d9e..7c088f99fd 100644
--- a/src/plugins/clearcase/clearcaseplugin.cpp
+++ b/src/plugins/clearcase/clearcaseplugin.cpp
@@ -218,7 +218,7 @@ public:
 
 protected:
     void updateActions(VcsBase::VcsBasePluginPrivate::ActionState) override;
-    bool submitEditorAboutToClose() override;
+    bool activateCommit() override;
     void discardCommit() override { cleanCheckInMessageFile(); }
     QString ccGet(const FilePath &workingDir, const QString &file, const QString &prefix = {});
     QList<QStringPair> ccGetActivities() const;
@@ -734,7 +734,7 @@ ClearCasePluginPrivate::ClearCasePluginPrivate()
 }
 
 // called before closing the submit editor
-bool ClearCasePluginPrivate::submitEditorAboutToClose()
+bool ClearCasePluginPrivate::activateCommit()
 {
     if (!isCheckInEditorOpen())
         return true;
diff --git a/src/plugins/cvs/cvsplugin.cpp b/src/plugins/cvs/cvsplugin.cpp
index 39cbf561fa..f6deac1088 100644
--- a/src/plugins/cvs/cvsplugin.cpp
+++ b/src/plugins/cvs/cvsplugin.cpp
@@ -238,7 +238,7 @@ public:
 
 protected:
     void updateActions(ActionState) final;
-    bool submitEditorAboutToClose() final;
+    bool activateCommit() final;
     void discardCommit() override { cleanCommitMessageFile(); }
 
 private:
@@ -706,7 +706,7 @@ void CvsPluginPrivate::vcsDescribe(const FilePath &source, const QString &change
         VcsOutputWindow::appendError(errorMessage);
 };
 
-bool CvsPluginPrivate::submitEditorAboutToClose()
+bool CvsPluginPrivate::activateCommit()
 {
     if (!isCommitEditorOpen())
         return true;
diff --git a/src/plugins/git/gitplugin.cpp b/src/plugins/git/gitplugin.cpp
index e269dc386e..1af164f247 100644
--- a/src/plugins/git/gitplugin.cpp
+++ b/src/plugins/git/gitplugin.cpp
@@ -250,7 +250,7 @@ public:
     void startRebaseFromCommit(const FilePath &workingDirectory, QString commit);
 
     void updateActions(VcsBasePluginPrivate::ActionState) override;
-    bool submitEditorAboutToClose() override;
+    bool activateCommit() override;
     void discardCommit() override { cleanCommitMessageFile(); }
 
     void diffCurrentFile();
@@ -1380,7 +1380,7 @@ IEditor *GitPluginPrivate::openSubmitEditor(const QString &fileName, const Commi
     return editor;
 }
 
-bool GitPluginPrivate::submitEditorAboutToClose()
+bool GitPluginPrivate::activateCommit()
 {
     if (!isCommitEditorOpen())
         return true;
diff --git a/src/plugins/mercurial/mercurialplugin.cpp b/src/plugins/mercurial/mercurialplugin.cpp
index e4882006f1..775c6c1f7e 100644
--- a/src/plugins/mercurial/mercurialplugin.cpp
+++ b/src/plugins/mercurial/mercurialplugin.cpp
@@ -136,7 +136,7 @@ public:
 
 private:
     void updateActions(VcsBase::VcsBasePluginPrivate::ActionState) final;
-    bool submitEditorAboutToClose() final;
+    bool activateCommit() final;
 
     // File menu action slots
     void addCurrentFile();
@@ -644,7 +644,7 @@ void MercurialPluginPrivate::diffFromEditorSelected(const QStringList &files)
     m_client.diff(m_submitRepository, files);
 }
 
-bool MercurialPluginPrivate::submitEditorAboutToClose()
+bool MercurialPluginPrivate::activateCommit()
 {
     auto commitEditor = qobject_cast<CommitEditor *>(submitEditor());
     QTC_ASSERT(commitEditor, return true);
diff --git a/src/plugins/perforce/perforceplugin.cpp b/src/plugins/perforce/perforceplugin.cpp
index 9033afe274..6ba890cf1e 100644
--- a/src/plugins/perforce/perforceplugin.cpp
+++ b/src/plugins/perforce/perforceplugin.cpp
@@ -222,7 +222,7 @@ public:
     void getTopLevel(const FilePath &workingDirectory = {}, bool isSync = false);
 
     void updateActions(ActionState) override;
-    bool submitEditorAboutToClose() override;
+    bool activateCommit() override;
     void discardCommit() override { cleanCommitMessageFile(); }
 
     QString commitDisplayName() const final;
@@ -1526,7 +1526,7 @@ bool PerforcePluginPrivate::isCommitEditorOpen() const
     return !m_commitMessageFileName.isEmpty();
 }
 
-bool PerforcePluginPrivate::submitEditorAboutToClose()
+bool PerforcePluginPrivate::activateCommit()
 {
     if (!isCommitEditorOpen())
         return true;
diff --git a/src/plugins/subversion/subversionplugin.cpp b/src/plugins/subversion/subversionplugin.cpp
index c11eb3901e..0925893ffb 100644
--- a/src/plugins/subversion/subversionplugin.cpp
+++ b/src/plugins/subversion/subversionplugin.cpp
@@ -222,7 +222,7 @@ public:
 
 protected:
     void updateActions(VcsBase::VcsBasePluginPrivate::ActionState) override;
-    bool submitEditorAboutToClose() override;
+    bool activateCommit() override;
     void discardCommit() override { cleanCommitMessageFile(); }
 
 private:
@@ -543,7 +543,7 @@ SubversionClient *SubversionPluginPrivate::client()
     return m_client;
 }
 
-bool SubversionPluginPrivate::submitEditorAboutToClose()
+bool SubversionPluginPrivate::activateCommit()
 {
     if (!isCommitEditorOpen())
         return true;
diff --git a/src/plugins/vcsbase/vcsbaseplugin.cpp b/src/plugins/vcsbase/vcsbaseplugin.cpp
index 7df56b3d62..a26872d864 100644
--- a/src/plugins/vcsbase/vcsbaseplugin.cpp
+++ b/src/plugins/vcsbase/vcsbaseplugin.cpp
@@ -503,7 +503,13 @@ VcsBasePluginPrivate::VcsBasePluginPrivate(const Context &context)
 {
     Internal::VcsPlugin *plugin = Internal::VcsPlugin::instance();
     connect(plugin, &Internal::VcsPlugin::submitEditorAboutToClose,
-            this, &VcsBasePluginPrivate::slotSubmitEditorAboutToClose);
+            this, [this](VcsBaseSubmitEditor *submitEditor, bool *result) {
+        if (submitEditor == m_submitEditor) {
+            *result = submitEditor->promptSubmit(this);
+            if (*result)
+                discardCommit();
+        }
+    });
     // First time: create new listener
     if (!m_listener)
         m_listener = new Internal::StateListener(plugin);
@@ -522,31 +528,6 @@ void VcsBasePluginPrivate::extensionsInitialized()
     m_listener->slotStateChanged();
 }
 
-void VcsBasePluginPrivate::slotSubmitEditorAboutToClose(VcsBaseSubmitEditor *submitEditor, bool *result)
-{
-    qCDebug(baseLog) << this << "plugin's submit editor" << m_submitEditor
-                     << (m_submitEditor ? m_submitEditor->document()->id().name() : QByteArray())
-                     << "closing submit editor" << submitEditor
-                     << (submitEditor ? submitEditor->document()->id().name() : QByteArray());
-    if (submitEditor == m_submitEditor) {
-        const VcsBaseSubmitEditor::PromptSubmitResult response = submitEditor->promptSubmit(this);
-        m_submitActionTriggered = false;
-
-        switch (response) {
-        case VcsBaseSubmitEditor::SubmitCanceled:
-            *result = false;
-            break;
-        case VcsBaseSubmitEditor::SubmitDiscarded:
-            discardCommit();
-            *result = true;
-            break;
-        default:
-            *result = submitEditorAboutToClose();
-            break;
-        }
-    }
-}
-
 void VcsBasePluginPrivate::slotStateChanged(const Internal::State &newInternalState, Core::IVersionControl *vc)
 {
     if (vc == this) {
@@ -614,10 +595,8 @@ QString VcsBasePluginPrivate::commitDisplayName() const
 
 void VcsBasePluginPrivate::commitFromEditor()
 {
-    // Close the submit editor
-    m_submitActionTriggered = true;
     QTC_ASSERT(m_submitEditor, return);
-    EditorManager::closeDocuments({m_submitEditor->document()});
+    m_submitEditor->accept(this);
 }
 
 bool VcsBasePluginPrivate::promptBeforeCommit()
diff --git a/src/plugins/vcsbase/vcsbaseplugin.h b/src/plugins/vcsbase/vcsbaseplugin.h
index 602444416c..e4ece349d0 100644
--- a/src/plugins/vcsbase/vcsbaseplugin.h
+++ b/src/plugins/vcsbase/vcsbaseplugin.h
@@ -137,7 +137,7 @@ public:
     virtual QString commitDisplayName() const;
 
     void commitFromEditor();
-    bool submitActionTriggered() const { return m_submitActionTriggered; }
+    virtual bool activateCommit() = 0;
 
 protected:
     // Prompt to save all files before commit:
@@ -164,8 +164,6 @@ protected:
 
     // Implement to enable the plugin menu actions according to state.
     virtual void updateActions(ActionState as) = 0;
-    // Implement to start the submit process, use submitEditor() to get the submit editor instance.
-    virtual bool submitEditorAboutToClose() = 0;
     virtual void discardCommit();
 
     // A helper to enable the VCS menu action according to state:
@@ -176,7 +174,6 @@ protected:
     bool enableMenuAction(ActionState as, QAction *in) const;
 
 private:
-    void slotSubmitEditorAboutToClose(VcsBaseSubmitEditor *submitEditor, bool *result);
     void slotStateChanged(const Internal::State &s, Core::IVersionControl *vc);
 
     bool supportsRepositoryCreation() const;
@@ -185,7 +182,6 @@ private:
     Core::Context m_context;
     VcsBasePluginState m_state;
     int m_actionState = -1;
-    bool m_submitActionTriggered = false;
 };
 
 } // namespace VcsBase
diff --git a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
index 1ed7a489e0..5e20b9eef5 100644
--- a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
+++ b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
@@ -125,6 +125,7 @@ public:
     QPointer<QAction> m_submitAction;
 
     NickNameDialog *m_nickNameDialog = nullptr;
+    bool m_disablePrompt = false;
 };
 
 VcsBaseSubmitEditorPrivate::VcsBaseSubmitEditorPrivate(SubmitEditorWidget *editorWidget,
@@ -478,46 +479,47 @@ void VcsBaseSubmitEditor::setDescriptionMandatory(bool v)
 
 enum { checkDialogMinimumWidth = 500 };
 
-VcsBaseSubmitEditor::PromptSubmitResult VcsBaseSubmitEditor::promptSubmit(VcsBasePluginPrivate *plugin)
+void VcsBaseSubmitEditor::accept(VcsBasePluginPrivate *plugin)
 {
     auto submitWidget = static_cast<SubmitEditorWidget *>(this->widget());
 
     Core::EditorManager::activateEditor(this, Core::EditorManager::IgnoreNavigationHistory);
 
-    if (!submitWidget->isEnabled() || !submitWidget->isEdited())
-        return SubmitDiscarded;
-
     QString errorMessage;
+    const bool canCommit = checkSubmitMessage(&errorMessage) && submitWidget->canSubmit(&errorMessage);
+    if (!canCommit) {
+        VcsOutputWindow::appendError(
+                    tr("Cannot %1%2.",
+                       "%2 is an optional error message with ': ' prefix. Don't add space in front.")
+                    .arg(plugin->commitDisplayName().toLower(),
+                         errorMessage.isEmpty() ? errorMessage : ": " + errorMessage));
+    } else if (plugin->activateCommit()) {
+        close();
+    }
+}
+
+void VcsBaseSubmitEditor::close()
+{
+    d->m_disablePrompt = true;
+    Core::EditorManager::closeDocuments({document()});
+}
+
+bool VcsBaseSubmitEditor::promptSubmit(VcsBasePluginPrivate *plugin)
+{
+    if (d->m_disablePrompt)
+        return true;
+
+    Core::EditorManager::activateEditor(this, Core::EditorManager::IgnoreNavigationHistory);
+
+    auto submitWidget = static_cast<SubmitEditorWidget *>(this->widget());
+    if (!submitWidget->isEnabled() || !submitWidget->isEdited())
+        return true;
 
-    // Pop up a message depending on whether the check succeeded and the
-    // user wants to be prompted
-    bool canCommit = checkSubmitMessage(&errorMessage) && submitWidget->canSubmit(&errorMessage);
-    const bool prompt = !plugin->submitActionTriggered();
-    if (canCommit && !prompt)
-        return SubmitConfirmed;
-    QMessageBox mb(Core::ICore::dialogParent());
     const QString commitName = plugin->commitDisplayName();
-    mb.setWindowTitle(tr("Close %1 %2 Editor").arg(plugin->displayName(), commitName));
-    mb.setIcon(QMessageBox::Question);
-    QString message;
-    if (canCommit) {
-        message = tr("What do you want to do with these changes?");
-    } else {
-        message = tr("Cannot %1%2.\nWhat do you want to do?",
-                     "%2 is an optional error message with ': ' prefix. Don't add space in front.")
-                .arg(commitName.toLower(),
-                     errorMessage.isEmpty() ? errorMessage : ": " + errorMessage);
-    }
-    mb.setText(message);
-    mb.setStandardButtons(QMessageBox::Close | QMessageBox::Cancel);
-    // On Windows there is no mnemonic for Close. Set it explicitly.
-    mb.button(QMessageBox::Close)->setText(tr("&Close"));
-    mb.button(QMessageBox::Cancel)->setText(tr("&Keep Editing"));
-    // prompt is true when the editor is closed, and false when triggered by the submit action
-    if (prompt)
-        mb.setDefaultButton(QMessageBox::Cancel);
-    mb.exec();
-    return mb.result() == QMessageBox::Close ? SubmitDiscarded : SubmitCanceled;
+    return QMessageBox::question(Core::ICore::dialogParent(),
+                                 tr("Close %1 %2 Editor").arg(plugin->displayName(), commitName),
+                                 tr("Closing this editor will abort the %1. Are you sure?")
+                                 .arg(commitName.toLower())) == QMessageBox::Yes;
 }
 
 QString VcsBaseSubmitEditor::promptForNickName()
diff --git a/src/plugins/vcsbase/vcsbasesubmiteditor.h b/src/plugins/vcsbase/vcsbasesubmiteditor.h
index 46e8469b6c..6e4ae28f5a 100644
--- a/src/plugins/vcsbase/vcsbasesubmiteditor.h
+++ b/src/plugins/vcsbase/vcsbasesubmiteditor.h
@@ -47,14 +47,8 @@ public:
 
     ~VcsBaseSubmitEditor() override;
 
-    // A utility routine to be called when closing a submit editor.
-    // Runs checks on the message and prompts according to configuration.
-    // Force prompt should be true if it is invoked by closing an editor
-    // as opposed to invoking the "Submit" button.
-    // 'promptSetting' points to a bool variable containing the plugin's
-    // prompt setting. The user can uncheck it from the message box.
-    enum PromptSubmitResult { SubmitConfirmed, SubmitCanceled, SubmitDiscarded };
-    PromptSubmitResult promptSubmit(VcsBasePluginPrivate *plugin);
+    void accept(VcsBasePluginPrivate *plugin);
+    bool promptSubmit(VcsBasePluginPrivate *plugin);
 
     QAbstractItemView::SelectionMode fileListSelectionMode() const;
     void setFileListSelectionMode(QAbstractItemView::SelectionMode sm);
@@ -123,6 +117,7 @@ private:
     bool checkSubmitMessage(QString *errorMessage) const;
     bool runSubmitMessageCheckScript(const QString &script, QString *errorMessage) const;
     QString promptForNickName();
+    void close();
 
     VcsBaseSubmitEditorPrivate *d = nullptr;