VCS: Improve dialog on submit prompt

The dialog appears when closing the commit dialog without committing,
or when "prompt to submit" setting is enabled.

Fixes: QTCREATORBUG-18799
Change-Id: I8eb20becbcee7281b9f673a35ec698c6f8e04a40
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@qt.io>
Reviewed-by: Robert Loehning <robert.loehning@qt.io>
Reviewed-by: André Hartmann <aha_1980@gmx.de>

9 files changed, 71 insertions, 89 deletions

diff --git a/src/plugins/bazaar/bazaarplugin.cpp b/src/plugins/bazaar/bazaarplugin.cpp
index 8048fa739f..a35b2ede62 100644
--- a/src/plugins/bazaar/bazaarplugin.cpp
+++ b/src/plugins/bazaar/bazaarplugin.cpp
@@ -635,11 +635,8 @@ bool BazaarPlugin::submitEditorAboutToClose()
     IDocument *editorDocument = commitEditor->document();
     QTC_ASSERT(editorDocument, return true);
 
-    bool dummyPrompt = false;
     const VcsBaseSubmitEditor::PromptSubmitResult response =
-            commitEditor->promptSubmit(tr("Close Commit Editor"), tr("Do you want to commit the changes?"),
-                                       tr("Message check failed. Do you want to proceed?"),
-                                       &dummyPrompt, !m_submitActionTriggered);
+            commitEditor->promptSubmit(this, nullptr, !m_submitActionTriggered);
     m_submitActionTriggered = false;
 
     switch (response) {
diff --git a/src/plugins/clearcase/clearcaseplugin.cpp b/src/plugins/clearcase/clearcaseplugin.cpp
index 96d17c2374..6223bedd4b 100644
--- a/src/plugins/clearcase/clearcaseplugin.cpp
+++ b/src/plugins/clearcase/clearcaseplugin.cpp
@@ -593,10 +593,7 @@ bool ClearCasePlugin::submitEditorAboutToClose()
     // is, the editor was closed or shutdown).
     bool prompt = m_settings.promptToCheckIn;
     const VcsBaseSubmitEditor::PromptSubmitResult answer =
-            editor->promptSubmit(tr("Closing ClearCase Editor"),
-                                 tr("Do you want to check in the files?"),
-                                 tr("The comment check failed. Do you want to check in the files?"),
-                                 &prompt, !m_submitActionTriggered);
+            editor->promptSubmit(this, &prompt, !m_submitActionTriggered);
     m_submitActionTriggered = false;
     switch (answer) {
     case VcsBaseSubmitEditor::SubmitCanceled:
diff --git a/src/plugins/cvs/cvsplugin.cpp b/src/plugins/cvs/cvsplugin.cpp
index 99c72a287d..7335ba53ff 100644
--- a/src/plugins/cvs/cvsplugin.cpp
+++ b/src/plugins/cvs/cvsplugin.cpp
@@ -431,12 +431,9 @@ bool CvsPlugin::submitEditorAboutToClose()
 
     // Prompt user. Force a prompt unless submit was actually invoked (that
     // is, the editor was closed or shutdown).
-    const VcsBaseSubmitEditor::PromptSubmitResult answer =
-            editor->promptSubmit(tr("Closing CVS Editor"),
-                                 tr("Do you want to commit the change?"),
-                                 tr("The commit message check failed. Do you want to commit the change?"),
-                                 client()->settings().boolPointer(CvsSettings::promptOnSubmitKey),
-                                 !m_submitActionTriggered);
+    const VcsBaseSubmitEditor::PromptSubmitResult answer = editor->promptSubmit(
+                this, client()->settings().boolPointer(CvsSettings::promptOnSubmitKey),
+                !m_submitActionTriggered);
     m_submitActionTriggered = false;
     switch (answer) {
     case VcsBaseSubmitEditor::SubmitCanceled:
diff --git a/src/plugins/git/gitplugin.cpp b/src/plugins/git/gitplugin.cpp
index f9dfa4a31c..8d2c54df9e 100644
--- a/src/plugins/git/gitplugin.cpp
+++ b/src/plugins/git/gitplugin.cpp
@@ -1034,12 +1034,8 @@ bool GitPlugin::submitEditorAboutToClose()
         return true;
     // Prompt user. Force a prompt unless submit was actually invoked (that
     // is, the editor was closed or shutdown).
-    bool promptData = false;
     const VcsBaseSubmitEditor::PromptSubmitResult answer
-            = editor->promptSubmit(tr("Closing Git Editor"),
-                 tr("Do you want to commit the change?"),
-                 tr("Git will not accept this commit. Do you want to continue to edit it?"),
-                 &promptData, !m_submitActionTriggered, false);
+            = editor->promptSubmit(this, nullptr, !m_submitActionTriggered, false);
     m_submitActionTriggered = false;
     switch (answer) {
     case VcsBaseSubmitEditor::SubmitCanceled:
diff --git a/src/plugins/mercurial/mercurialplugin.cpp b/src/plugins/mercurial/mercurialplugin.cpp
index 662586b7a8..80ff8907f5 100644
--- a/src/plugins/mercurial/mercurialplugin.cpp
+++ b/src/plugins/mercurial/mercurialplugin.cpp
@@ -551,11 +551,8 @@ bool MercurialPlugin::submitEditorAboutToClose()
     Core::IDocument *editorFile = commitEditor->document();
     QTC_ASSERT(editorFile, return true);
 
-    bool dummyPrompt = false;
     const VcsBaseSubmitEditor::PromptSubmitResult response =
-            commitEditor->promptSubmit(tr("Close Commit Editor"), tr("Do you want to commit the changes?"),
-                                       tr("Message check failed. Do you want to proceed?"),
-                                       &dummyPrompt, !m_submitActionTriggered);
+            commitEditor->promptSubmit(this, nullptr, !m_submitActionTriggered);
     m_submitActionTriggered = false;
 
     switch (response) {
diff --git a/src/plugins/perforce/perforceplugin.cpp b/src/plugins/perforce/perforceplugin.cpp
index 1ad1cf7dc5..5c43f44320 100644
--- a/src/plugins/perforce/perforceplugin.cpp
+++ b/src/plugins/perforce/perforceplugin.cpp
@@ -1310,10 +1310,7 @@ bool PerforcePlugin::submitEditorAboutToClose()
     // is, the editor was closed or shutdown).
     bool wantsPrompt = m_settings.promptToSubmit();
     const VcsBaseSubmitEditor::PromptSubmitResult answer =
-            perforceEditor->promptSubmit(tr("Closing p4 Editor"),
-                                         tr("Do you want to submit this change list?"),
-                                         tr("The commit message check failed. Do you want to submit this change list?"),
-                                         &wantsPrompt, !m_submitActionTriggered);
+            perforceEditor->promptSubmit(this, &wantsPrompt, !m_submitActionTriggered);
     m_submitActionTriggered = false;
 
     if (answer == VcsBaseSubmitEditor::SubmitCanceled)
diff --git a/src/plugins/subversion/subversionplugin.cpp b/src/plugins/subversion/subversionplugin.cpp
index 66b671d136..5c585ee264 100644
--- a/src/plugins/subversion/subversionplugin.cpp
+++ b/src/plugins/subversion/subversionplugin.cpp
@@ -425,12 +425,9 @@ bool SubversionPlugin::submitEditorAboutToClose()
     // Prompt user. Force a prompt unless submit was actually invoked (that
     // is, the editor was closed or shutdown).
     VcsBaseClientSettings &newSettings = client()->settings();
-    const VcsBaseSubmitEditor::PromptSubmitResult answer =
-            editor->promptSubmit(tr("Closing Subversion Editor"),
-                                 tr("Do you want to commit the change?"),
-                                 tr("The commit message check failed. Do you want to commit the change?"),
-                                 newSettings.boolPointer(SubversionSettings::promptOnSubmitKey),
-                                 !m_submitActionTriggered);
+    const VcsBaseSubmitEditor::PromptSubmitResult answer = editor->promptSubmit(
+                this, newSettings.boolPointer(SubversionSettings::promptOnSubmitKey),
+                !m_submitActionTriggered);
     m_submitActionTriggered = false;
     switch (answer) {
     case VcsBaseSubmitEditor::SubmitCanceled:
diff --git a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
index 91a6932a46..157d16cf3b 100644
--- a/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
+++ b/src/plugins/vcsbase/vcsbasesubmiteditor.cpp
@@ -31,6 +31,7 @@
 #include "submiteditorwidget.h"
 #include "submitfieldwidget.h"
 #include "submitfilemodel.h"
+#include "vcsbaseplugin.h"
 #include "vcsoutputwindow.h"
 #include "vcsplugin.h"
 #include "vcsprojectcache.h"
@@ -58,6 +59,7 @@
 #include <QFileInfo>
 #include <QPointer>
 #include <QProcess>
+#include <QPushButton>
 #include <QSet>
 #include <QStringListModel>
 #include <QStyle>
@@ -523,14 +525,31 @@ void VcsBaseSubmitEditor::setDescriptionMandatory(bool v)
 
 enum { checkDialogMinimumWidth = 500 };
 
+static QString withUnusedMnemonic(QString string, const QList<QPushButton *> &otherButtons)
+{
+    QSet<QChar> mnemonics;
+    for (QPushButton *button : otherButtons) {
+        const QString text = button->text();
+        const int ampersandPos = text.indexOf('&');
+        if (ampersandPos >= 0 && ampersandPos < text.size() - 1)
+            mnemonics.insert(text.at(ampersandPos + 1));
+    }
+    for (int i = 0, total = string.length(); i < total; ++i) {
+        if (!mnemonics.contains(string.at(i)))
+            return string.insert(i, '&');
+    }
+    return string;
+}
+
 VcsBaseSubmitEditor::PromptSubmitResult
-        VcsBaseSubmitEditor::promptSubmit(const QString &title,
-                                          const QString &question,
-                                          const QString &checkFailureQuestion,
+        VcsBaseSubmitEditor::promptSubmit(VcsBasePlugin *plugin,
                                           bool *promptSetting,
                                           bool forcePrompt,
                                           bool canCommitOnFailure)
 {
+    bool dummySetting = false;
+    if (!promptSetting)
+        promptSetting = &dummySetting;
     auto submitWidget = static_cast<SubmitEditorWidget *>(this->widget());
 
     Core::EditorManager::activateEditor(this, Core::EditorManager::IgnoreNavigationHistory);
@@ -539,67 +558,52 @@ VcsBaseSubmitEditor::PromptSubmitResult
         return SubmitDiscarded;
 
     QString errorMessage;
-    QMessageBox::StandardButton answer = QMessageBox::Yes;
 
     const bool prompt = forcePrompt || *promptSetting;
 
-    QWidget *parent = Core::ICore::mainWindow();
     // Pop up a message depending on whether the check succeeded and the
     // user wants to be prompted
     bool canCommit = checkSubmitMessage(&errorMessage) && submitWidget->canSubmit(&errorMessage);
+    if (canCommit && !prompt)
+        return SubmitConfirmed;
+    CheckableMessageBox mb(Core::ICore::dialogParent());
+    const QString commitName = plugin->commitDisplayName();
+    mb.setWindowTitle(tr("Close %1 %2 Editor")
+                      .arg(plugin->versionControl()->displayName(), commitName));
+    mb.setIconPixmap(QMessageBox::standardIcon(QMessageBox::Question));
+    QString message;
     if (canCommit) {
-        // Check ok, do prompt?
-        if (prompt) {
-            // Provide check box to turn off prompt ONLY if it was not forced
-            if (*promptSetting && !forcePrompt) {
-                const QDialogButtonBox::StandardButton danswer =
-                        CheckableMessageBox::question(parent, title, question,
-                                                                   tr("Prompt to submit"), promptSetting,
-                                                                   QDialogButtonBox::Yes|QDialogButtonBox::No|
-                                                                   QDialogButtonBox::Cancel,
-                                                                   QDialogButtonBox::Yes);
-                answer = CheckableMessageBox::dialogButtonBoxToMessageBoxButton(danswer);
-            } else {
-                answer = QMessageBox::question(parent, title, question,
-                                               QMessageBox::Yes|QMessageBox::No|QMessageBox::Cancel,
-                                               QMessageBox::Yes);
-            }
-        }
+        message = tr("What do you want to do with these changes?");
     } else {
-        // Check failed.
-        QMessageBox::StandardButtons buttons;
-        if (canCommitOnFailure)
-            buttons = QMessageBox::Yes|QMessageBox::No|QMessageBox::Cancel;
-        else
-            buttons = QMessageBox::Yes|QMessageBox::No;
-        QMessageBox msgBox(QMessageBox::Question, title, checkFailureQuestion,
-                           buttons, parent);
-        msgBox.setDefaultButton(QMessageBox::Cancel);
-        msgBox.setInformativeText(errorMessage);
-        msgBox.setMinimumWidth(checkDialogMinimumWidth);
-        answer = static_cast<QMessageBox::StandardButton>(msgBox.exec());
+        message = tr("Cannot %1%2.\nWhat do you want to do?",
+                     "%2 is an optional error message with ': ' prefix. Do not separate it from %1.")
+                .arg(commitName.toLower(),
+                     errorMessage.isEmpty() ? errorMessage : ": " + errorMessage);
     }
-    if (!canCommit && !canCommitOnFailure) {
-        switch (answer) {
-        case QMessageBox::No:
-            return SubmitDiscarded;
-        case QMessageBox::Yes:
-            return SubmitCanceled;
-        default:
-            break;
-        }
-    } else {
-        switch (answer) {
-        case QMessageBox::No:
-            return SubmitDiscarded;
-        case QMessageBox::Yes:
-            return SubmitConfirmed;
-        default:
-            break;
-        }
+    mb.setText(message);
+    mb.setCheckBoxText(tr("Prompt to %1").arg(commitName.toLower()));
+    mb.setChecked(*promptSetting);
+    // Provide check box to turn off prompt ONLY if it was not forced
+    mb.setCheckBoxVisible(*promptSetting && !forcePrompt);
+    QDialogButtonBox::StandardButtons buttons = QDialogButtonBox::Close | QDialogButtonBox::Cancel;
+    if (canCommit || canCommitOnFailure)
+        buttons |= QDialogButtonBox::Ok;
+    mb.setStandardButtons(buttons);
+    QPushButton *cancelButton = mb.button(QDialogButtonBox::Cancel);
+    cancelButton->setText(tr("&Keep Editing"));
+    cancelButton->setDefault(true);
+    if (QPushButton *commitButton = mb.button(QDialogButtonBox::Ok)) {
+        commitButton->setText(withUnusedMnemonic(commitName,
+                              {cancelButton, mb.button(QDialogButtonBox::Close)}));
     }
-
-    return SubmitCanceled;
+    if (mb.exec() == QDialog::Accepted)
+        *promptSetting = mb.isChecked();
+    QAbstractButton *chosen = mb.clickedButton();
+    if (!chosen || chosen == cancelButton)
+        return SubmitCanceled;
+    if (chosen == mb.button(QDialogButtonBox::Close))
+        return SubmitDiscarded;
+    return SubmitConfirmed;
 }
 
 QString VcsBaseSubmitEditor::promptForNickName()
diff --git a/src/plugins/vcsbase/vcsbasesubmiteditor.h b/src/plugins/vcsbase/vcsbasesubmiteditor.h
index 5e2a2ce242..223fa6f7f5 100644
--- a/src/plugins/vcsbase/vcsbasesubmiteditor.h
+++ b/src/plugins/vcsbase/vcsbasesubmiteditor.h
@@ -46,6 +46,7 @@ namespace Internal {
 
 class SubmitEditorWidget;
 class SubmitFileModel;
+class VcsBasePlugin;
 class VcsBaseSubmitEditorPrivate;
 
 class VCSBASE_EXPORT VcsBaseSubmitEditorParameters
@@ -84,8 +85,7 @@ public:
     // '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(const QString &title, const QString &question,
-                                    const QString &checkFailureQuestion,
+    PromptSubmitResult promptSubmit(VcsBasePlugin *plugin,
                                     bool *promptSetting,
                                     bool forcePrompt = false,
                                     bool canCommitOnFailure = true);