Fix repeting Stage/Unstage actions in unified diff editor

Don't store diff file index, chunk index and diff editor
controller anymore. Pass needed indices by value,
retrieve them when needed as lambda copy.

Change-Id: I3a81f1ab6d131c0b1d9899ac4b061b6e25582f51
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

src/plugins/diffeditor/diffeditorcontroller.cpp

@@ -71,9 +71,10 @@ QString DiffEditorController::revisionFromDescription() const
     return m_document->description().mid(7, 12);
 }
 
-QString DiffEditorController::makePatch(PatchOptions options) const
+QString DiffEditorController::makePatch(int fileIndex, int chunkIndex,
+                                        PatchOptions options) const
 {
-    return m_document->makePatch(m_diffFileIndex, m_chunkIndex,
+    return m_document->makePatch(fileIndex, chunkIndex,
                                  options & Revert, options & AddPrefix);
 }
 
@@ -152,11 +153,27 @@ void DiffEditorController::reloadFinished(bool success)
     m_isReloading = false;
 }
 
-void DiffEditorController::requestChunkActions(QMenu *menu, int diffFileIndex, int chunkIndex)
+void DiffEditorController::requestChunkActions(QMenu *menu, int fileIndex, int chunkIndex)
 {
-    m_diffFileIndex = diffFileIndex;
+    emit chunkActionsRequested(menu, fileIndex, chunkIndex);
-    m_chunkIndex = chunkIndex;
+}
-    emit chunkActionsRequested(menu, diffFileIndex >= 0 && chunkIndex >= 0);
+
+bool DiffEditorController::chunkExists(int fileIndex, int chunkIndex) const
+{
+    if (!m_document)
+        return false;
+
+    if (fileIndex < 0 || chunkIndex < 0)
+        return false;
+
+    if (fileIndex >= m_document->diffFiles().count())
+        return false;
+
+    const FileData fileData = m_document->diffFiles().at(fileIndex);
+    if (chunkIndex >= fileData.chunks.count())
+        return false;
+
+    return true;
 }
 
 } // namespace DiffEditor

src/plugins/diffeditor/diffeditorcontroller.h

@@ -59,17 +59,18 @@ public:
         AddPrefix = 2
     };
     Q_DECLARE_FLAGS(PatchOptions, PatchOption)
-    QString makePatch(PatchOptions options) const;
+    QString makePatch(int fileIndex, int chunkIndex, PatchOptions options) const;
 
     static Core::IDocument *findOrCreateDocument(const QString &vcsId,
                                                  const QString &displayName);
     static DiffEditorController *controller(Core::IDocument *document);
 
     void branchesReceived(const QString &branches);
-    void requestChunkActions(QMenu *menu, int diffFileIndex, int chunkIndex);
+    void requestChunkActions(QMenu *menu, int fileIndex, int chunkIndex);
+    bool chunkExists(int fileIndex, int chunkIndex) const;
 
 signals:
-    void chunkActionsRequested(QMenu *menu, bool isValid);
+    void chunkActionsRequested(QMenu *menu, int fileIndex, int chunkIndex);
     void requestInformationForCommit(const QString &revision);
 
 protected:
@@ -92,10 +93,10 @@ private:
     Internal::DiffEditorDocument *const m_document;
 
     bool m_isReloading = false;
-    int m_diffFileIndex = -1;
-    int m_chunkIndex = -1;
 
     friend class Internal::DiffEditorDocument;
 };
 
+Q_DECLARE_OPERATORS_FOR_FLAGS(DiffEditorController::PatchOptions)
+
 } // namespace DiffEditor

src/plugins/diffeditor/diffeditorwidgetcontroller.cpp

@@ -123,11 +123,14 @@ void DiffEditorWidgetController::hideProgress()
         m_progressIndicator->hide();
 }
 
-void DiffEditorWidgetController::patch(bool revert)
+void DiffEditorWidgetController::patch(bool revert, int fileIndex, int chunkIndex)
 {
     if (!m_document)
         return;
 
+    if (!chunkExists(fileIndex, chunkIndex))
+        return;
+
     const QString title = revert ? tr("Revert Chunk") : tr("Apply Chunk");
     const QString question = revert
             ? tr("Would you like to revert the chunk?")
@@ -139,7 +142,7 @@ void DiffEditorWidgetController::patch(bool revert)
         return;
     }
 
-    const FileData fileData = m_contextFileData.at(m_contextMenuFileIndex);
+    const FileData fileData = m_contextFileData.at(fileIndex);
     const QString fileName = revert
             ? fileData.rightFileInfo.fileName
             : fileData.leftFileInfo.fileName;
@@ -155,7 +158,7 @@ void DiffEditorWidgetController::patch(bool revert)
     if (patchBehaviour == DiffFileInfo::PatchFile) {
         const int strip = m_document->baseDirectory().isEmpty() ? -1 : 0;
 
-        const QString patch = m_document->makePatch(m_contextMenuFileIndex, m_contextMenuChunkIndex, revert);
+        const QString patch = m_document->makePatch(fileIndex, chunkIndex, revert);
 
         if (patch.isEmpty())
             return;
@@ -180,8 +183,8 @@ void DiffEditorWidgetController::patch(bool revert)
         const QString contentsCopyFileName = contentsCopy.fileName();
         const QString contentsCopyDir = QFileInfo(contentsCopyFileName).absolutePath();
 
-        const QString patch = m_document->makePatch(m_contextMenuFileIndex,
+        const QString patch = m_document->makePatch(fileIndex,
-                              m_contextMenuChunkIndex, revert, false, QFileInfo(contentsCopyFileName).fileName());
+                              chunkIndex, revert, false, QFileInfo(contentsCopyFileName).fileName());
 
         if (patch.isEmpty())
             return;
@@ -219,65 +222,59 @@ void DiffEditorWidgetController::setFontSettings(const FontSettings &fontSetting
     m_rightCharFormat = fontSettings.toTextCharFormat(C_DIFF_DEST_CHAR);
 }
 
-void DiffEditorWidgetController::addCodePasterAction(QMenu *menu)
+void DiffEditorWidgetController::addCodePasterAction(QMenu *menu, int fileIndex, int chunkIndex)
 {
     if (ExtensionSystem::PluginManager::getObject<CodePaster::Service>()) {
         // optional code pasting service
         QAction *sendChunkToCodePasterAction = menu->addAction(tr("Send Chunk to CodePaster..."));
-        connect(sendChunkToCodePasterAction, &QAction::triggered,
+        connect(sendChunkToCodePasterAction, &QAction::triggered, [this, fileIndex, chunkIndex]() {
-                this, &DiffEditorWidgetController::slotSendChunkToCodePaster);
+            sendChunkToCodePaster(fileIndex, chunkIndex);
+        });
     }
 }
 
-bool DiffEditorWidgetController::setAndVerifyIndexes(QMenu *menu,
+bool DiffEditorWidgetController::chunkExists(int fileIndex, int chunkIndex) const
-                                                     int diffFileIndex, int chunkIndex)
 {
     if (!m_document)
         return false;
 
-    m_contextMenuFileIndex = diffFileIndex;
-    m_contextMenuChunkIndex = chunkIndex;
-
-    if (m_contextMenuFileIndex < 0 || m_contextMenuChunkIndex < 0)
-        return false;
-
-    if (m_contextMenuFileIndex >= m_contextFileData.count())
-        return false;
-
-    const FileData fileData = m_contextFileData.at(m_contextMenuFileIndex);
-    if (m_contextMenuChunkIndex >= fileData.chunks.count())
-        return false;
-
     if (DiffEditorController *controller = m_document->controller())
-        controller->requestChunkActions(menu, diffFileIndex, chunkIndex);
+        return controller->chunkExists(fileIndex, chunkIndex);
 
-    return true;
+    return false;
 }
 
-bool DiffEditorWidgetController::fileNamesAreDifferent() const
+bool DiffEditorWidgetController::fileNamesAreDifferent(int fileIndex) const
 {
-    const FileData fileData = m_contextFileData.at(m_contextMenuFileIndex);
+    const FileData fileData = m_contextFileData.at(fileIndex);
     return fileData.leftFileInfo.fileName != fileData.rightFileInfo.fileName;
 }
 
-void DiffEditorWidgetController::addApplyAction(QMenu *menu, int diffFileIndex,
+void DiffEditorWidgetController::addApplyAction(QMenu *menu, int fileIndex, int chunkIndex)
-                                                int chunkIndex)
 {
     QAction *applyAction = menu->addAction(tr("Apply Chunk..."));
-    connect(applyAction, &QAction::triggered, this, &DiffEditorWidgetController::slotApplyChunk);
+    connect(applyAction, &QAction::triggered, [this, fileIndex, chunkIndex]() {
-    applyAction->setEnabled(setAndVerifyIndexes(menu, diffFileIndex, chunkIndex)
+        patch(false, fileIndex, chunkIndex);
-                            && fileNamesAreDifferent());
+    });
+    applyAction->setEnabled(chunkExists(fileIndex, chunkIndex) && fileNamesAreDifferent(fileIndex));
 }
 
-void DiffEditorWidgetController::addRevertAction(QMenu *menu, int diffFileIndex,
+void DiffEditorWidgetController::addRevertAction(QMenu *menu, int fileIndex, int chunkIndex)
-                                                 int chunkIndex)
 {
     QAction *revertAction = menu->addAction(tr("Revert Chunk..."));
-    connect(revertAction, &QAction::triggered, this, &DiffEditorWidgetController::slotRevertChunk);
+    connect(revertAction, &QAction::triggered, [this, fileIndex, chunkIndex]() {
-    revertAction->setEnabled(setAndVerifyIndexes(menu, diffFileIndex, chunkIndex));
+        patch(true, fileIndex, chunkIndex);
+    });
+    revertAction->setEnabled(chunkExists(fileIndex, chunkIndex));
 }
 
-void DiffEditorWidgetController::slotSendChunkToCodePaster()
+void DiffEditorWidgetController::addExtraActions(QMenu *menu, int fileIndex, int chunkIndex)
+{
+    if (DiffEditorController *controller = m_document->controller())
+        controller->requestChunkActions(menu, fileIndex, chunkIndex);
+}
+
+void DiffEditorWidgetController::sendChunkToCodePaster(int fileIndex, int chunkIndex)
 {
     if (!m_document)
         return;
@@ -286,7 +283,7 @@ void DiffEditorWidgetController::slotSendChunkToCodePaster()
     auto pasteService = ExtensionSystem::PluginManager::getObject<CodePaster::Service>();
     QTC_ASSERT(pasteService, return);
 
-    const QString patch = m_document->makePatch(m_contextMenuFileIndex, m_contextMenuChunkIndex, false);
+    const QString patch = m_document->makePatch(fileIndex, chunkIndex, false);
 
     if (patch.isEmpty())
         return;
@@ -294,15 +291,5 @@ void DiffEditorWidgetController::slotSendChunkToCodePaster()
     pasteService->postText(patch, Constants::DIFF_EDITOR_MIMETYPE);
 }
 
-void DiffEditorWidgetController::slotApplyChunk()
-{
-    patch(false);
-}
-
-void DiffEditorWidgetController::slotRevertChunk()
-{
-    patch(true);
-}
-
 } // namespace Internal
 } // namespace DiffEditor

src/plugins/diffeditor/diffeditorwidgetcontroller.h

@@ -52,13 +52,13 @@ public:
     void setDocument(DiffEditorDocument *document);
     DiffEditorDocument *document() const;
 
-    void patch(bool revert);
     void jumpToOriginalFile(const QString &fileName, int lineNumber,
                             int columnNumber);
     void setFontSettings(const TextEditor::FontSettings &fontSettings);
-    void addCodePasterAction(QMenu *menu);
+    void addCodePasterAction(QMenu *menu, int fileIndex, int chunkIndex);
-    void addApplyAction(QMenu *menu, int diffFileIndex, int chunkIndex);
+    void addApplyAction(QMenu *menu, int fileIndex, int chunkIndex);
-    void addRevertAction(QMenu *menu, int diffFileIndex, int chunkIndex);
+    void addRevertAction(QMenu *menu, int fileIndex, int chunkIndex);
+    void addExtraActions(QMenu *menu, int fileIndex, int chunkIndex);
 
     bool m_ignoreCurrentIndexChange = false;
     QList<FileData> m_contextFileData; // ultimate data to be shown
@@ -71,11 +71,10 @@ public:
     QTextCharFormat m_rightCharFormat;
 
 private:
-    void slotSendChunkToCodePaster();
+    void patch(bool revert, int fileIndex, int chunkIndex);
-    void slotApplyChunk();
+    void sendChunkToCodePaster(int fileIndex, int chunkIndex);
-    void slotRevertChunk();
+    bool chunkExists(int fileIndex, int chunkIndex) const;
-    bool setAndVerifyIndexes(QMenu *menu, int diffFileIndex, int chunkIndex);
+    bool fileNamesAreDifferent(int fileIndex) const;
-    bool fileNamesAreDifferent() const;
 
     void scheduleShowProgress();
     void showProgress();
@@ -85,9 +84,6 @@ private:
 
     DiffEditorDocument *m_document = nullptr;
 
-    int m_contextMenuFileIndex = -1;
-    int m_contextMenuChunkIndex = -1;
-
     Utils::ProgressIndicator *m_progressIndicator = nullptr;
     QTimer m_timer;
 };

src/plugins/diffeditor/sidebysidediffeditorwidget.cpp

@@ -1059,23 +1059,25 @@ void SideBySideDiffEditorWidget::slotRightJumpToOriginalFileRequested(
 }
 
 void SideBySideDiffEditorWidget::slotLeftContextMenuRequested(QMenu *menu,
-                                                              int diffFileIndex,
+                                                              int fileIndex,
                                                               int chunkIndex)
 {
     menu->addSeparator();
 
-    m_controller.addCodePasterAction(menu);
+    m_controller.addCodePasterAction(menu, fileIndex, chunkIndex);
-    m_controller.addApplyAction(menu, diffFileIndex, chunkIndex);
+    m_controller.addApplyAction(menu, fileIndex, chunkIndex);
+    m_controller.addExtraActions(menu, fileIndex, chunkIndex);
 }
 
 void SideBySideDiffEditorWidget::slotRightContextMenuRequested(QMenu *menu,
-                                                               int diffFileIndex,
+                                                               int fileIndex,
                                                                int chunkIndex)
 {
     menu->addSeparator();
 
-    m_controller.addCodePasterAction(menu);
+    m_controller.addCodePasterAction(menu, fileIndex, chunkIndex);
-    m_controller.addRevertAction(menu, diffFileIndex, chunkIndex);
+    m_controller.addRevertAction(menu, fileIndex, chunkIndex);
+    m_controller.addExtraActions(menu, fileIndex, chunkIndex);
 }
 
 void SideBySideDiffEditorWidget::leftVSliderChanged()

src/plugins/diffeditor/sidebysidediffeditorwidget.h

@@ -85,9 +85,9 @@ private:
                                              int lineNumber, int columnNumber);
     void slotRightJumpToOriginalFileRequested(int diffFileIndex,
                                               int lineNumber, int columnNumber);
-    void slotLeftContextMenuRequested(QMenu *menu, int diffFileIndex,
+    void slotLeftContextMenuRequested(QMenu *menu, int fileIndex,
                                       int chunkIndex);
-    void slotRightContextMenuRequested(QMenu *menu, int diffFileIndex,
+    void slotRightContextMenuRequested(QMenu *menu, int fileIndex,
                                        int chunkIndex);
     void leftVSliderChanged();
     void rightVSliderChanged();

src/plugins/diffeditor/unifieddiffeditorwidget.cpp

@@ -190,14 +190,15 @@ void UnifiedDiffEditorWidget::contextMenuEvent(QContextMenuEvent *e)
 }
 
 void UnifiedDiffEditorWidget::addContextMenuActions(QMenu *menu,
-                                                    int diffFileIndex,
+                                                    int fileIndex,
                                                     int chunkIndex)
 {
     menu->addSeparator();
 
-    m_controller.addCodePasterAction(menu);
+    m_controller.addCodePasterAction(menu, fileIndex, chunkIndex);
-    m_controller.addApplyAction(menu, diffFileIndex, chunkIndex);
+    m_controller.addApplyAction(menu, fileIndex, chunkIndex);
-    m_controller.addRevertAction(menu, diffFileIndex, chunkIndex);
+    m_controller.addRevertAction(menu, fileIndex, chunkIndex);
+    m_controller.addExtraActions(menu, fileIndex, chunkIndex);
 }
 
 void UnifiedDiffEditorWidget::clear(const QString &message)

src/plugins/diffeditor/unifieddiffeditorwidget.h

@@ -96,7 +96,7 @@ private:
     int chunkIndexForBlockNumber(int blockNumber) const;
     void jumpToOriginalFile(const QTextCursor &cursor);
     void addContextMenuActions(QMenu *menu,
-                               int diffFileIndex,
+                               int fileIndex,
                                int chunkIndex);
 
     // block number, visual line number.

src/plugins/git/gitclient.cpp

@@ -596,56 +596,49 @@ QTextCodec *GitClient::codecFor(GitClient::CodecType codecType, const QString &s
     return nullptr;
 }
 
-void GitClient::slotChunkActionsRequested(QMenu *menu, bool isValid)
+void GitClient::chunkActionsRequested(QMenu *menu, int fileIndex, int chunkIndex)
 {
+    QPointer<DiffEditor::DiffEditorController> diffController
+            = qobject_cast<DiffEditorController *>(sender());
+
+    auto stageChunk = [this](QPointer<DiffEditor::DiffEditorController> diffController,
+            int fileIndex, int chunkIndex, bool revert) {
+        if (diffController.isNull())
+            return;
+
+        DiffEditorController::PatchOptions options = DiffEditorController::AddPrefix;
+        if (revert)
+            options |= DiffEditorController::Revert;
+        const QString patch = diffController->makePatch(fileIndex, chunkIndex, options);
+        stage(diffController, patch, revert);
+    };
+
     menu->addSeparator();
     QAction *stageChunkAction = menu->addAction(tr("Stage Chunk"));
-    connect(stageChunkAction, &QAction::triggered, this, &GitClient::slotStageChunk);
+    connect(stageChunkAction, &QAction::triggered,
+            [this, stageChunk, diffController, fileIndex, chunkIndex]() {
+        stageChunk(diffController, fileIndex, chunkIndex, false);
+    });
     QAction *unstageChunkAction = menu->addAction(tr("Unstage Chunk"));
-    connect(unstageChunkAction, &QAction::triggered, this, &GitClient::slotUnstageChunk);
+    connect(unstageChunkAction, &QAction::triggered,
+            [this, stageChunk, diffController, fileIndex, chunkIndex]() {
+        stageChunk(diffController, fileIndex, chunkIndex, true);
+    });
 
-    m_contextController = qobject_cast<DiffEditorController *>(sender());
+    if (!diffController || !diffController->chunkExists(fileIndex, chunkIndex)) {
-
-    if (!isValid || !m_contextController) {
         stageChunkAction->setEnabled(false);
         unstageChunkAction->setEnabled(false);
     }
 }
 
-void GitClient::slotStageChunk()
+void GitClient::stage(DiffEditor::DiffEditorController *diffController,
-{
+                      const QString &patch, bool revert)
-    if (m_contextController.isNull())
-        return;
-
-    DiffEditorController::PatchOptions options = DiffEditorController::AddPrefix;
-    const QString patch = m_contextController->makePatch(options);
-    if (patch.isEmpty())
-        return;
-
-    stage(patch, false);
-}
-
-void GitClient::slotUnstageChunk()
-{
-    if (m_contextController.isNull())
-        return;
-
-    DiffEditorController::PatchOptions options = DiffEditorController::AddPrefix;
-    options |= DiffEditorController::Revert;
-    const QString patch = m_contextController->makePatch(options);
-    if (patch.isEmpty())
-        return;
-
-    stage(patch, true);
-}
-
-void GitClient::stage(const QString &patch, bool revert)
 {
     Utils::TemporaryFile patchFile("git-patchfile");
     if (!patchFile.open())
         return;
 
-    const QString baseDir = m_contextController->baseDirectory();
+    const QString baseDir = diffController->baseDirectory();
     QTextCodec *codec = EditorManager::defaultTextCodec();
     const QByteArray patchData = codec
             ? codec->fromUnicode(patch) : patch.toLocal8Bit();
@@ -666,7 +659,7 @@ void GitClient::stage(const QString &patch, bool revert)
         } else {
             VcsOutputWindow::appendError(errorMessage);
         }
-        m_contextController->requestReload();
+        diffController->requestReload();
     } else {
         VcsOutputWindow::appendError(errorMessage);
     }
@@ -685,7 +678,7 @@ void GitClient::requestReload(const QString &documentId, const QString &source,
     QTC_ASSERT(controller, return);
 
     connect(controller, &DiffEditorController::chunkActionsRequested,
-            this, &GitClient::slotChunkActionsRequested, Qt::DirectConnection);
+            this, &GitClient::chunkActionsRequested, Qt::DirectConnection);
     connect(controller, &DiffEditorController::requestInformationForCommit,
             this, &GitClient::branchesForCommit);
 

src/plugins/git/gitclient.h

@@ -329,12 +329,11 @@ public:
 
 private:
     void finishSubmoduleUpdate();
-    void slotChunkActionsRequested(QMenu *menu, bool isValid);
+    void chunkActionsRequested(QMenu *menu, int fileIndex, int chunkIndex);
-    void slotStageChunk();
-    void slotUnstageChunk();
     void branchesForCommit(const QString &revision);
 
-    void stage(const QString &patch, bool revert);
+    void stage(DiffEditor::DiffEditorController *diffController,
+               const QString &patch, bool revert);
 
     enum CodecType { CodecSource, CodecLogOutput, CodecNone };
     QTextCodec *codecFor(CodecType codecType, const QString &source = QString()) const;
@@ -378,7 +377,6 @@ private:
     QMap<QString, StashInfo> m_stashInfo;
     QStringList m_updatedSubmodules;
     bool m_disableEditor;
-    QPointer<DiffEditor::DiffEditorController> m_contextController;
     QFutureSynchronizer<void> m_synchronizer; // for commit updates
 };