VcsCommand: Limit the usage of setCookie()

Don't use setCookie() for setting the push failure status.
Introduce a PushHandler helper that keeps the data collected
when stdErrText() signal is emitted and reuse it when
finished() is being called. Move m_pushFallbackCommand
field into the helper, as its value is meaningful only
in context of the invoked command.

Hide PushFailure enum inside PushHandler.

Change-Id: Id21457734f32e06bb130c3c07c24cb2b7af6e466
Reviewed-by: Orgad Shaneh <orgads@gmail.com>

src/plugins/git/gitclient.cpp

@@ -3290,21 +3290,22 @@ void GitClient::subversionDeltaCommit(const FilePath &workingDirectory) const
     vcsExec(workingDirectory, {"svn", "dcommit"}, nullptr, true, VcsCommand::ShowSuccessMessage);
 }
 
-void GitClient::push(const FilePath &workingDirectory, const QStringList &pushArgs)
+class PushHandler : public QObject
 {
-    VcsCommand *command = vcsExec(workingDirectory, QStringList({"push"}) + pushArgs, nullptr, true,
+public:
-                                  VcsCommand::ShowSuccessMessage);
+    PushHandler(GitClient *gitClient, const FilePath &workingDir, const QStringList &pushArgs)
-    connect(command, &VcsCommand::stdErrText, this, [this, command](const QString &text) {
+        : m_gitClient(gitClient)
-        PushFailure failure = Unknown;
+    {
-        if (text.contains("non-fast-forward"))
+        VcsCommand *command = gitClient->vcsExec(workingDir, QStringList({"push"}) + pushArgs,
-            failure = NonFastForward;
+                                                 nullptr, true, VcsCommand::ShowSuccessMessage);
-        else if (text.contains("has no upstream branch"))
+        // Make command a parent of this in order to delete this when command is deleted
-            failure = NoRemoteBranch;
+        setParent(command);
 
-        if (failure != Unknown)
+        connect(command, &VcsCommand::stdErrText, this, [this](const QString &text) {
-            command->setCookie(failure);
+            if (text.contains("non-fast-forward")) {
-
+                m_pushFailure = NonFastForward;
-        if (failure == NoRemoteBranch) {
+            } else if (text.contains("has no upstream branch")) {
+                m_pushFailure = NoRemoteBranch;
                 const QStringList lines = text.split('\n', Qt::SkipEmptyParts);
                 for (const QString &line : lines) {
                     /* Extract the suggested command from the git output which
@@ -3320,16 +3321,16 @@ void GitClient::push(const FilePath &workingDirectory, const QStringList &pushAr
                 }
             }
         });
-    connect(command, &VcsCommand::finished, this,
+
-            [this, workingDirectory, pushArgs](bool success, const QVariant &cookie) {
+        connect(command, &VcsCommand::finished, this, [this, workingDir, pushArgs](bool success) {
             if (success) {
                 GitPlugin::updateCurrentBranch();
                 return;
             }
-        switch (static_cast<PushFailure>(cookie.toInt())) {
+            if (m_pushFailure == Unknown || !m_gitClient)
-        case Unknown:
+                return;
-            break;
+
-        case NonFastForward: {
+            if (m_pushFailure == NonFastForward) {
                 const QColor warnColor = Utils::creatorTheme()->color(Theme::TextColorError);
                 if (QMessageBox::question(
                             Core::ICore::dialogParent(), tr("Force Push"),
@@ -3337,42 +3338,49 @@ void GitClient::push(const FilePath &workingDirectory, const QStringList &pushAr
                                "(rewrites remote history)</span>?")
                             .arg(QString::number(warnColor.rgba(), 16)),
                             QMessageBox::Yes | QMessageBox::No,
-                        QMessageBox::No) == QMessageBox::Yes) {
+                            QMessageBox::No) != QMessageBox::Yes) {
-                VcsCommand *rePushCommand = vcsExec(workingDirectory,
+                    return;
+                }
+                VcsCommand *rePushCommand = m_gitClient->vcsExec(workingDir,
                            QStringList({"push", "--force-with-lease"}) + pushArgs,
                            nullptr, true, VcsCommand::ShowSuccessMessage);
                 connect(rePushCommand, &VcsCommand::finished, this, [](bool success) {
                     if (success)
                         GitPlugin::updateCurrentBranch();
                 });
+                return;
             }
-            break;
+            // NoRemoteBranch case
-        }
-        case NoRemoteBranch:
             if (QMessageBox::question(
                         Core::ICore::dialogParent(), tr("No Upstream Branch"),
                         tr("Push failed because the local branch \"%1\" "
                            "does not have an upstream branch on the remote.\n\n"
                            "Would you like to create the branch \"%1\" on the "
                            "remote and set it as upstream?")
-                        .arg(synchronousCurrentLocalBranch(workingDirectory)),
+                        .arg(m_gitClient->synchronousCurrentLocalBranch(workingDir)),
                         QMessageBox::Yes | QMessageBox::No,
-                        QMessageBox::No) == QMessageBox::Yes) {
+                        QMessageBox::No) != QMessageBox::Yes) {
-
+                return;
+            }
             const QStringList fallbackCommandParts =
                     m_pushFallbackCommand.split(' ', Qt::SkipEmptyParts);
-                VcsCommand *rePushCommand = vcsExec(workingDirectory,
+            VcsCommand *rePushCommand = m_gitClient->vcsExec(workingDir,
-                                                    fallbackCommandParts.mid(1), nullptr, true,
+                       fallbackCommandParts.mid(1), nullptr, true, VcsCommand::ShowSuccessMessage);
-                                                    VcsCommand::ShowSuccessMessage);
+            connect(rePushCommand, &VcsCommand::finished, this, [workingDir](bool success) {
-                connect(rePushCommand, &VcsCommand::finished, this,
-                        [workingDirectory](bool success) {
                 if (success)
-                        GitPlugin::updateBranches(workingDirectory);
+                    GitPlugin::updateBranches(workingDir);
+            });
         });
     }
-            break;
+private:
-        }
+    enum PushFailure { Unknown, NonFastForward, NoRemoteBranch } m_pushFailure = Unknown;
-    });
+    QPointer<GitClient> m_gitClient;
+    QString m_pushFallbackCommand;
+};
+
+void GitClient::push(const FilePath &workingDirectory, const QStringList &pushArgs)
+{
+    new PushHandler(this, workingDirectory, pushArgs);
 }
 
 bool GitClient::synchronousMerge(const FilePath &workingDirectory, const QString &branch,

src/plugins/git/gitclient.h

@@ -80,12 +80,6 @@ enum StashFlag {
     NoPrompt       = 0x02
 };
 
-enum PushFailure {
-    Unknown,
-    NonFastForward,
-    NoRemoteBranch
-};
-
 class SubmoduleData
 {
 public:
@@ -425,7 +419,6 @@ private:
 
     QString m_gitQtcEditor;
     QMap<Utils::FilePath, StashInfo> m_stashInfo;
-    QString m_pushFallbackCommand;
     QString m_diffCommit;
     Utils::FilePaths m_updatedSubmodules;
     bool m_disableEditor = false;