Git: refactoring ChangeSelectionDialog

> Dialog now combines show, cherry-pick and revert
> has fixed path, and no way to change it
> not created on stack

Change-Id: I7cee0b2e775a80941b51a4ca023064baf0d6575c
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
Reviewed-by: Petar Perisin <petar.perisin@gmail.com>
Reviewed-by: Tobias Hunger <tobias.hunger@digia.com>

src/plugins/git/changeselectiondialog.cpp

@@ -31,9 +31,15 @@
 #include "gitplugin.h"
 #include "gitclient.h"
 
-#include <QFileDialog>
-#include <QMessageBox>
 #include <QProcess>
+#include <QFormLayout>
+#include <QSpacerItem>
+#include <QFormLayout>
+#include <QHBoxLayout>
+#include <QPushButton>
+#include <QLabel>
+#include <QLineEdit>
+#include <QPlainTextEdit>
 
 namespace Git {
 namespace Internal {
@@ -41,27 +47,53 @@ namespace Internal {
 ChangeSelectionDialog::ChangeSelectionDialog(const QString &workingDirectory, QWidget *parent)
     : QDialog(parent)
     , m_process(0)
+    , m_workingDirectoryLabel(new QLabel(workingDirectory, this))
+    , m_changeNumberEdit(new QLineEdit(this))
+    , m_detailsText(new QPlainTextEdit(this))
+    , m_showButton(new QPushButton(tr("Show"), this))
+    , m_cherryPickButton(new QPushButton(tr("Cherry Pick"), this))
+    , m_revertButton(new QPushButton(tr("Revert"), this))
+    , m_cancelButton(new QPushButton(tr("Cancel"), this))
+    , m_command(NoCommand)
 {
-    m_ui.setupUi(this);
-    if (!workingDirectory.isEmpty()) {
-        setWorkingDirectory(workingDirectory);
-        m_ui.workingDirectoryButton->setEnabled(false);
-        m_ui.workingDirectoryEdit->setEnabled(false);
-    }
-    setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
-    connect(m_ui.workingDirectoryButton, SIGNAL(clicked()), this, SLOT(selectWorkingDirectory()));
-    setWindowTitle(tr("Select a Git Commit"));
-
     bool ok;
     m_gitBinaryPath = GitPlugin::instance()->gitClient()->gitBinaryPath(&ok);
     if (!ok)
         return;
 
+    QGridLayout* layout = new QGridLayout(this);
+    layout->addWidget(new QLabel(tr("Working Directory:"), this), 0, 0 , 1, 1);
+    layout->addWidget(m_workingDirectoryLabel, 0, 1, 1, 1);
+    layout->addWidget(new QLabel(tr("Change:"), this),1, 0, 1, 1);
+    layout->addWidget(m_changeNumberEdit, 1, 1, 1, 1);
+    layout->addWidget(m_detailsText, 2, 0, 1, 2);
+
+    QHBoxLayout* buttonsLine = new QHBoxLayout();
+    buttonsLine->addWidget(m_cancelButton);
+    buttonsLine->addStretch();
+    buttonsLine->addWidget(m_revertButton);
+    buttonsLine->addWidget(m_cherryPickButton);
+    buttonsLine->addWidget(m_showButton);
+
+    layout->addLayout(buttonsLine, 3, 0 ,1 ,2);
+
+    m_changeNumberEdit->setFocus(Qt::ActiveWindowFocusReason);
+    m_changeNumberEdit->setText(QLatin1String("HEAD"));
+    m_changeNumberEdit->selectAll();
+
+    setWindowTitle(tr("Select a Git Commit"));
+    setGeometry(0, 0, 550, 350);
+    adjustPosition(parent);
+
     m_gitEnvironment = GitPlugin::instance()->gitClient()->processEnvironment();
-    connect(m_ui.changeNumberEdit, SIGNAL(textChanged(QString)),
+    connect(m_changeNumberEdit, SIGNAL(textChanged(QString)),
             this, SLOT(recalculateDetails(QString)));
-    connect(m_ui.workingDirectoryEdit, SIGNAL(textChanged(QString)), this, SLOT(refresh()));
-    refresh();
+    connect(m_showButton, SIGNAL(clicked()), this, SLOT(acceptShow()));
+    connect(m_cherryPickButton, SIGNAL(clicked()), this, SLOT(acceptCherryPick()));
+    connect(m_revertButton, SIGNAL(clicked()), this, SLOT(acceptRevert()));
+    connect(m_cancelButton, SIGNAL(clicked()), this, SLOT(reject()));
+
+    recalculateDetails(m_changeNumberEdit->text());
 }
 
 ChangeSelectionDialog::~ChangeSelectionDialog()
@@ -71,49 +103,53 @@ ChangeSelectionDialog::~ChangeSelectionDialog()
 
 QString ChangeSelectionDialog::change() const
 {
-    return m_ui.changeNumberEdit->text();
+    return m_changeNumberEdit->text();
 }
 
 QString ChangeSelectionDialog::workingDirectory() const
 {
-    return m_ui.workingDirectoryEdit->text();
+    return m_workingDirectoryLabel->text();
 }
 
-void ChangeSelectionDialog::setWorkingDirectory(const QString &s)
+ChangeCommand ChangeSelectionDialog::command() const
 {
-    if (s.isEmpty())
-        return;
-    m_ui.workingDirectoryEdit->setText(QDir::toNativeSeparators(s));
-    m_ui.changeNumberEdit->setFocus(Qt::ActiveWindowFocusReason);
-    m_ui.changeNumberEdit->setText(QLatin1String("HEAD"));
-    m_ui.changeNumberEdit->selectAll();
+    return m_command;
 }
 
-void ChangeSelectionDialog::selectWorkingDirectory()
+void ChangeSelectionDialog::acceptCherryPick()
 {
-    QString location = QFileDialog::getExistingDirectory(this, tr("Select Working Directory"),
-                                                         m_ui.workingDirectoryEdit->text());
-    if (location.isEmpty())
-        return;
+    m_command = CherryPick;
+    accept();
+}
 
-    // Verify that the location is a repository
-    // We allow specifying a directory, which is not the head directory of the repository.
-    // This is useful for git show commit:./file
-    QString topLevel = GitPlugin::instance()->gitClient()->findRepositoryForDirectory(location);
-    if (!topLevel.isEmpty())
-        m_ui.workingDirectoryEdit->setText(location);
-    else // Did not find a repo
-        QMessageBox::critical(this, tr("Error"),
-                              tr("Selected directory is not a Git repository."));
+void ChangeSelectionDialog::acceptRevert()
+{
+    m_command = Revert;
+    accept();
+}
+
+void ChangeSelectionDialog::acceptShow()
+{
+    m_command = Show;
+    accept();
 }
 
 //! Set commit message in details
 void ChangeSelectionDialog::setDetails(int exitCode)
 {
-    if (exitCode == 0)
-        m_ui.detailsText->setPlainText(QString::fromUtf8(m_process->readAllStandardOutput()));
-    else
-        m_ui.detailsText->setPlainText(tr("Error: Unknown reference"));
+    if (exitCode == 0) {
+        m_detailsText->setPlainText(QString::fromUtf8(m_process->readAllStandardOutput()));
+        enableButtons(true);
+    } else {
+        m_detailsText->setPlainText(tr("Error: Unknown reference"));
+    }
+}
+
+void ChangeSelectionDialog::enableButtons(bool b)
+{
+    m_showButton->setEnabled(b);
+    m_cherryPickButton->setEnabled(b);
+    m_revertButton->setEnabled(b);
 }
 
 void ChangeSelectionDialog::recalculateDetails(const QString &ref)
@@ -123,6 +159,7 @@ void ChangeSelectionDialog::recalculateDetails(const QString &ref)
         m_process->waitForFinished();
         delete m_process;
     }
+    enableButtons(false);
 
     QStringList args;
     args << QLatin1String("log") << QLatin1String("-n1") << ref;
@@ -136,14 +173,9 @@ void ChangeSelectionDialog::recalculateDetails(const QString &ref)
     m_process->start(m_gitBinaryPath, args);
     m_process->closeWriteChannel();
     if (!m_process->waitForStarted())
-        m_ui.detailsText->setPlainText(tr("Error: Could not start Git."));
+        m_detailsText->setPlainText(tr("Error: Could not start Git."));
     else
-        m_ui.detailsText->setPlainText(tr("Fetching commit data..."));
-}
-
-void ChangeSelectionDialog::refresh()
-{
-    recalculateDetails(m_ui.changeNumberEdit->text());
+        m_detailsText->setPlainText(tr("Fetching commit data..."));
 }
 
 } // Internal

src/plugins/git/changeselectiondialog.h

@@ -32,38 +32,60 @@
 
 #include <QDialog>
 #include <QProcessEnvironment>
-QT_FORWARD_DECLARE_CLASS(QProcess)
 
-#include "ui_changeselectiondialog.h"
+QT_BEGIN_NAMESPACE
+class QPushButton;
+class QLabel;
+class QLineEdit;
+class QPlainTextEdit;
+class QProcess;
+QT_END_NAMESPACE
 
 namespace Git {
 namespace Internal {
 
+enum ChangeCommand {
+    NoCommand,
+    CherryPick,
+    Revert,
+    Show
+};
+
 class ChangeSelectionDialog : public QDialog
 {
     Q_OBJECT
 public:
-    ChangeSelectionDialog(const QString &workingDirectory = QString(), QWidget *parent = 0);
+    ChangeSelectionDialog(const QString &workingDirectory, QWidget *parent);
     ~ChangeSelectionDialog();
 
     QString change() const;
 
     QString workingDirectory() const;
-    void setWorkingDirectory(const QString &s);
-
-public slots:
-    void selectWorkingDirectory();
+    ChangeCommand command() const;
 
 private slots:
     void setDetails(int exitCode);
     void recalculateDetails(const QString &ref);
-    void refresh();
+    void acceptCherryPick();
+    void acceptRevert();
+    void acceptShow();
 
 private:
-    Ui_ChangeSelectionDialog m_ui;
+    void enableButtons(bool b);
+
     QProcess* m_process;
     QString m_gitBinaryPath;
     QProcessEnvironment m_gitEnvironment;
+
+    QLabel* m_workingDirectoryLabel;
+    QLineEdit* m_changeNumberEdit;
+    QPlainTextEdit* m_detailsText;
+    QPushButton* m_showButton;
+    QPushButton* m_cherryPickButton;
+    QPushButton* m_revertButton;
+    QPushButton* m_cancelButton;
+
+    ChangeCommand m_command;
 };
 
 } // namespace Internal

src/plugins/git/changeselectiondialog.ui

@@ -1,104 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<ui version="4.0">
- <class>Git::Internal::ChangeSelectionDialog</class>
- <widget class="QDialog" name="Git::Internal::ChangeSelectionDialog">
-  <property name="geometry">
-   <rect>
-    <x>0</x>
-    <y>0</y>
-    <width>595</width>
-    <height>396</height>
-   </rect>
-  </property>
-  <layout class="QGridLayout" name="gridLayout">
-   <property name="sizeConstraint">
-    <enum>QLayout::SetDefaultConstraint</enum>
-   </property>
-   <item row="0" column="0">
-    <widget class="QLabel" name="workingDirectoryLabel">
-     <property name="text">
-      <string>Working directory:</string>
-     </property>
-    </widget>
-   </item>
-   <item row="0" column="1">
-    <widget class="QLineEdit" name="workingDirectoryEdit"/>
-   </item>
-   <item row="0" column="2">
-    <widget class="QPushButton" name="workingDirectoryButton">
-     <property name="text">
-      <string>Select</string>
-     </property>
-    </widget>
-   </item>
-   <item row="1" column="0">
-    <widget class="QLabel" name="changeLabel">
-     <property name="text">
-      <string>Change:</string>
-     </property>
-    </widget>
-   </item>
-   <item row="1" column="1" colspan="2">
-    <widget class="QLineEdit" name="changeNumberEdit"/>
-   </item>
-   <item row="2" column="0" colspan="3">
-    <widget class="QPlainTextEdit" name="detailsText">
-     <property name="undoRedoEnabled">
-      <bool>false</bool>
-     </property>
-     <property name="lineWrapMode">
-      <enum>QPlainTextEdit::NoWrap</enum>
-     </property>
-     <property name="readOnly">
-      <bool>true</bool>
-     </property>
-    </widget>
-   </item>
-   <item row="3" column="0" colspan="3">
-    <widget class="QDialogButtonBox" name="buttonBox">
-     <property name="orientation">
-      <enum>Qt::Horizontal</enum>
-     </property>
-     <property name="standardButtons">
-      <set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
-     </property>
-    </widget>
-   </item>
-  </layout>
- </widget>
- <resources/>
- <connections>
-  <connection>
-   <sender>buttonBox</sender>
-   <signal>accepted()</signal>
-   <receiver>Git::Internal::ChangeSelectionDialog</receiver>
-   <slot>accept()</slot>
-   <hints>
-    <hint type="sourcelabel">
-     <x>248</x>
-     <y>254</y>
-    </hint>
-    <hint type="destinationlabel">
-     <x>157</x>
-     <y>274</y>
-    </hint>
-   </hints>
-  </connection>
-  <connection>
-   <sender>buttonBox</sender>
-   <signal>rejected()</signal>
-   <receiver>Git::Internal::ChangeSelectionDialog</receiver>
-   <slot>reject()</slot>
-   <hints>
-    <hint type="sourcelabel">
-     <x>316</x>
-     <y>260</y>
-    </hint>
-    <hint type="destinationlabel">
-     <x>286</x>
-     <y>274</y>
-    </hint>
-   </hints>
-  </connection>
- </connections>
-</ui>

src/plugins/git/git.pro

@@ -48,8 +48,7 @@ SOURCES += gitplugin.cpp \
     mergetool.cpp \
     branchcheckoutdialog.cpp
 
-FORMS += changeselectiondialog.ui \
-    settingspage.ui \
+FORMS += settingspage.ui \
     gitsubmitpanel.ui \
     branchdialog.ui \
     stashdialog.ui \

src/plugins/git/git.qbs

@@ -28,7 +28,6 @@ QtcPlugin {
         "branchmodel.h",
         "changeselectiondialog.cpp",
         "changeselectiondialog.h",
-        "changeselectiondialog.ui",
         "clonewizard.cpp",
         "clonewizard.h",
         "clonewizardpage.cpp",

src/plugins/git/gitplugin.cpp

@@ -130,7 +130,6 @@ GitPlugin *GitPlugin::m_instance = 0;
 GitPlugin::GitPlugin() :
     VcsBase::VcsBasePlugin(Git::Constants::GITSUBMITEDITOR_ID),
     m_commandLocator(0),
-    m_showAction(0),
     m_submitCurrentAction(0),
     m_diffSelectedFilesAction(0),
     m_undoAction(0),
@@ -138,7 +137,6 @@ GitPlugin::GitPlugin() :
     m_menuAction(0),
     m_applyCurrentFilePatchAction(0),
     m_gitClient(0),
-    m_changeSelectionDialog(0),
     m_submitActionTriggered(false)
 {
     m_instance = this;
@@ -440,12 +438,8 @@ bool GitPlugin::initialize(const QStringList &arguments, QString *errorMessage)
                            globalcontext, true, SLOT(startRebase()));
 
     createRepositoryAction(localRepositoryMenu,
-                           tr("Revert Single Commit..."), Core::Id("Git.Revert"),
-                           globalcontext, true, SLOT(startRevertCommit()));
-
-    createRepositoryAction(localRepositoryMenu,
-                           tr("Cherry-Pick Commit..."), Core::Id("Git.CherryPick"),
-                           globalcontext, true, SLOT(startCherryPickCommit()));
+                           tr("Change-related Actions..."), Core::Id("Git.ChangeRelatedActions"),
+                           globalcontext, true, SLOT(startChangeRelatedAction()));
 
     // --------------
     localRepositoryMenu->addSeparator(globalcontext);
@@ -589,11 +583,6 @@ bool GitPlugin::initialize(const QStringList &arguments, QString *errorMessage)
     // --------------
     gitContainer->addSeparator(globalcontext);
 
-    m_showAction
-            = createRepositoryAction(gitContainer,
-                                     tr("Show..."), Core::Id("Git.ShowCommit"),
-                                     globalcontext, true, SLOT(showCommit())).first;
-
     m_createRepositoryAction = new QAction(tr("Create Repository..."), this);
     Core::Command *createRepositoryCommand = Core::ActionManager::registerAction(m_createRepositoryAction, "Git.CreateRepository", globalcontext);
     connect(m_createRepositoryAction, SIGNAL(triggered()), this, SLOT(createRepository()));
@@ -741,40 +730,51 @@ void GitPlugin::startRebase()
         m_gitClient->interactiveRebase(workingDirectory, change);
 }
 
-void GitPlugin::startRevertCommit()
+void GitPlugin::startChangeRelatedAction()
 {
     const VcsBase::VcsBasePluginState state = currentState();
-    QString workingDirectory = state.currentDirectoryOrTopLevel();
+    const QString workingDirectory = state.currentDirectoryOrTopLevel();
     if (workingDirectory.isEmpty())
         return;
-    GitClient::StashGuard stashGuard(workingDirectory, QLatin1String("Revert"));
+
+    QPointer<ChangeSelectionDialog> dialog = new ChangeSelectionDialog
+            (workingDirectory, Core::ICore::mainWindow());
+
+    int result = dialog->exec();
+
+    if (dialog.isNull() || (result == QDialog::Rejected) || dialog->change().isEmpty())
+        return;
+
+    const QString change = dialog->change();
+
+    if (dialog->command() == Show) {
+        m_gitClient->show(workingDirectory, change);
+        return;
+    }
+
+    QString command;
+    bool (GitClient::*commandFunction)(const QString&, const QString&);
+    switch (dialog->command()) {
+    case CherryPick:
+        command = QLatin1String("Cherry-pick");
+        commandFunction = &GitClient::cherryPickCommit;
+        break;
+    case Revert:
+        command = QLatin1String("Revert");
+        commandFunction = &GitClient::revertCommit;
+        break;
+    default:
+        return;
+    }
+
+    GitClient::StashGuard stashGuard(workingDirectory, command);
     if (stashGuard.stashingFailed(true))
         return;
-    ChangeSelectionDialog changeSelectionDialog(workingDirectory);
 
-    if (changeSelectionDialog.exec() != QDialog::Accepted)
-        return;
-    const QString change = changeSelectionDialog.change();
-    if (!change.isEmpty() && !m_gitClient->revertCommit(workingDirectory, change))
+    if (!(m_gitClient->*commandFunction)(workingDirectory, change))
         stashGuard.preventPop();
-}
 
-void GitPlugin::startCherryPickCommit()
-{
-    const VcsBase::VcsBasePluginState state = currentState();
-    QString workingDirectory = state.currentDirectoryOrTopLevel();
-    if (workingDirectory.isEmpty())
-        return;
-    GitClient::StashGuard stashGuard(state.topLevel(), QLatin1String("Cherry-pick"));
-    if (stashGuard.stashingFailed(true))
-        return;
-    ChangeSelectionDialog changeSelectionDialog(workingDirectory);
-
-    if (changeSelectionDialog.exec() != QDialog::Accepted)
-        return;
-    const QString change = changeSelectionDialog.change();
-    if (!change.isEmpty() && !m_gitClient->cherryPickCommit(workingDirectory, change))
-        stashGuard.preventPop();
+    delete dialog;
 }
 
 void GitPlugin::stageFile()
@@ -1228,9 +1228,6 @@ void GitPlugin::updateActions(VcsBase::VcsBasePlugin::ActionState as)
     foreach (QAction *repositoryAction, m_repositoryActions)
         repositoryAction->setEnabled(repositoryEnabled);
     updateRepositoryBrowserAction();
-
-    // Prompts for repo.
-    m_showAction->setEnabled(true);
 }
 
 void GitPlugin::updateRepositoryBrowserAction()
@@ -1240,24 +1237,6 @@ void GitPlugin::updateRepositoryBrowserAction()
     m_repositoryBrowserAction->setEnabled(repositoryEnabled && hasRepositoryBrowserCmd);
 }
 
-void GitPlugin::showCommit()
-{
-    const VcsBase::VcsBasePluginState state = currentState();
-
-    if (!m_changeSelectionDialog)
-        m_changeSelectionDialog = new ChangeSelectionDialog();
-
-    m_changeSelectionDialog->setWorkingDirectory(state.currentDirectoryOrTopLevel());
-
-    if (m_changeSelectionDialog->exec() != QDialog::Accepted)
-        return;
-    const QString change = m_changeSelectionDialog->change();
-    if (change.isEmpty())
-        return;
-
-    m_gitClient->show(m_changeSelectionDialog->workingDirectory(), change);
-}
-
 const GitSettings &GitPlugin::settings() const
 {
     return m_settings;

src/plugins/git/gitplugin.h

@@ -68,7 +68,6 @@ namespace Internal {
 
 class GitVersionControl;
 class GitClient;
-class ChangeSelectionDialog;
 class GitSubmitEditor;
 class CommitData;
 class StashDialog;
@@ -117,8 +116,7 @@ private slots:
     void undoUnstagedFileChanges();
     void resetRepository();
     void startRebase();
-    void startRevertCommit();
-    void startCherryPickCommit();
+    void startChangeRelatedAction();
     void stageFile();
     void unstageFile();
     void gitkForCurrentFile();
@@ -129,7 +127,6 @@ private slots:
     void promptApplyPatch();
     void gitClientMemberFuncRepositoryAction();
 
-    void showCommit();
     void startAmendCommit();
     void stash();
     void stashSnapshot();
@@ -201,8 +198,6 @@ private:
     Locator::CommandLocator *m_commandLocator;
     QAction *m_createRepositoryAction;
 
-    QAction *m_showAction;
-
     QAction *m_submitCurrentAction;
     QAction *m_diffSelectedFilesAction;
     QAction *m_undoAction;
@@ -216,7 +211,6 @@ private:
     Utils::ParameterAction *m_applyCurrentFilePatchAction;
 
     GitClient                   *m_gitClient;
-    ChangeSelectionDialog       *m_changeSelectionDialog;
     QPointer<StashDialog>       m_stashDialog;
     QPointer<BranchDialog>      m_branchDialog;
     QPointer<RemoteDialog>      m_remoteDialog;