From 64233a4fae6de690a3e4ec1ffd7a85496cdea644 Mon Sep 17 00:00:00 2001 From: Jarek Kobus Date: Thu, 15 Feb 2018 10:29:14 +0100 Subject: [PATCH] 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 --- .../diffeditor/diffeditorcontroller.cpp | 29 +++++-- src/plugins/diffeditor/diffeditorcontroller.h | 11 +-- .../diffeditor/diffeditorwidgetcontroller.cpp | 83 ++++++++----------- .../diffeditor/diffeditorwidgetcontroller.h | 20 ++--- .../diffeditor/sidebysidediffeditorwidget.cpp | 14 ++-- .../diffeditor/sidebysidediffeditorwidget.h | 4 +- .../diffeditor/unifieddiffeditorwidget.cpp | 9 +- .../diffeditor/unifieddiffeditorwidget.h | 2 +- src/plugins/git/gitclient.cpp | 67 +++++++-------- src/plugins/git/gitclient.h | 8 +- 10 files changed, 121 insertions(+), 126 deletions(-) diff --git a/src/plugins/diffeditor/diffeditorcontroller.cpp b/src/plugins/diffeditor/diffeditorcontroller.cpp index c64da4562d7..bb2738b985d 100644 --- a/src/plugins/diffeditor/diffeditorcontroller.cpp +++ b/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; - m_chunkIndex = chunkIndex; - emit chunkActionsRequested(menu, diffFileIndex >= 0 && chunkIndex >= 0); + emit chunkActionsRequested(menu, fileIndex, chunkIndex); +} + +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 diff --git a/src/plugins/diffeditor/diffeditorcontroller.h b/src/plugins/diffeditor/diffeditorcontroller.h index 643a855f80a..e5d5ca74184 100644 --- a/src/plugins/diffeditor/diffeditorcontroller.h +++ b/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 diff --git a/src/plugins/diffeditor/diffeditorwidgetcontroller.cpp b/src/plugins/diffeditor/diffeditorwidgetcontroller.cpp index 1e7b4bc35c4..c4a826bfe90 100644 --- a/src/plugins/diffeditor/diffeditorwidgetcontroller.cpp +++ b/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, - m_contextMenuChunkIndex, revert, false, QFileInfo(contentsCopyFileName).fileName()); + const QString patch = m_document->makePatch(fileIndex, + 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()) { // optional code pasting service QAction *sendChunkToCodePasterAction = menu->addAction(tr("Send Chunk to CodePaster...")); - connect(sendChunkToCodePasterAction, &QAction::triggered, - this, &DiffEditorWidgetController::slotSendChunkToCodePaster); + connect(sendChunkToCodePasterAction, &QAction::triggered, [this, fileIndex, chunkIndex]() { + sendChunkToCodePaster(fileIndex, chunkIndex); + }); } } -bool DiffEditorWidgetController::setAndVerifyIndexes(QMenu *menu, - int diffFileIndex, int chunkIndex) +bool DiffEditorWidgetController::chunkExists(int fileIndex, int chunkIndex) const { 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, - int chunkIndex) +void DiffEditorWidgetController::addApplyAction(QMenu *menu, int fileIndex, int chunkIndex) { QAction *applyAction = menu->addAction(tr("Apply Chunk...")); - connect(applyAction, &QAction::triggered, this, &DiffEditorWidgetController::slotApplyChunk); - applyAction->setEnabled(setAndVerifyIndexes(menu, diffFileIndex, chunkIndex) - && fileNamesAreDifferent()); + connect(applyAction, &QAction::triggered, [this, fileIndex, chunkIndex]() { + patch(false, fileIndex, chunkIndex); + }); + applyAction->setEnabled(chunkExists(fileIndex, chunkIndex) && fileNamesAreDifferent(fileIndex)); } -void DiffEditorWidgetController::addRevertAction(QMenu *menu, int diffFileIndex, - int chunkIndex) +void DiffEditorWidgetController::addRevertAction(QMenu *menu, int fileIndex, int chunkIndex) { QAction *revertAction = menu->addAction(tr("Revert Chunk...")); - connect(revertAction, &QAction::triggered, this, &DiffEditorWidgetController::slotRevertChunk); - revertAction->setEnabled(setAndVerifyIndexes(menu, diffFileIndex, chunkIndex)); + connect(revertAction, &QAction::triggered, [this, fileIndex, 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(); 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 diff --git a/src/plugins/diffeditor/diffeditorwidgetcontroller.h b/src/plugins/diffeditor/diffeditorwidgetcontroller.h index f5b605936c2..c0c1be95486 100644 --- a/src/plugins/diffeditor/diffeditorwidgetcontroller.h +++ b/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 addApplyAction(QMenu *menu, int diffFileIndex, int chunkIndex); - void addRevertAction(QMenu *menu, int diffFileIndex, int chunkIndex); + void addCodePasterAction(QMenu *menu, int fileIndex, int chunkIndex); + void addApplyAction(QMenu *menu, int fileIndex, int chunkIndex); + void addRevertAction(QMenu *menu, int fileIndex, int chunkIndex); + void addExtraActions(QMenu *menu, int fileIndex, int chunkIndex); bool m_ignoreCurrentIndexChange = false; QList m_contextFileData; // ultimate data to be shown @@ -71,11 +71,10 @@ public: QTextCharFormat m_rightCharFormat; private: - void slotSendChunkToCodePaster(); - void slotApplyChunk(); - void slotRevertChunk(); - bool setAndVerifyIndexes(QMenu *menu, int diffFileIndex, int chunkIndex); - bool fileNamesAreDifferent() const; + void patch(bool revert, int fileIndex, int chunkIndex); + void sendChunkToCodePaster(int fileIndex, int chunkIndex); + bool chunkExists(int fileIndex, int chunkIndex) const; + bool fileNamesAreDifferent(int fileIndex) 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; }; diff --git a/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp b/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp index fb1fe91e640..81942a9a665 100644 --- a/src/plugins/diffeditor/sidebysidediffeditorwidget.cpp +++ b/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.addApplyAction(menu, diffFileIndex, chunkIndex); + m_controller.addCodePasterAction(menu, fileIndex, 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.addRevertAction(menu, diffFileIndex, chunkIndex); + m_controller.addCodePasterAction(menu, fileIndex, chunkIndex); + m_controller.addRevertAction(menu, fileIndex, chunkIndex); + m_controller.addExtraActions(menu, fileIndex, chunkIndex); } void SideBySideDiffEditorWidget::leftVSliderChanged() diff --git a/src/plugins/diffeditor/sidebysidediffeditorwidget.h b/src/plugins/diffeditor/sidebysidediffeditorwidget.h index ba5c8ef550d..c315cec975c 100644 --- a/src/plugins/diffeditor/sidebysidediffeditorwidget.h +++ b/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(); diff --git a/src/plugins/diffeditor/unifieddiffeditorwidget.cpp b/src/plugins/diffeditor/unifieddiffeditorwidget.cpp index dbea4cbfb9c..54ca275a581 100644 --- a/src/plugins/diffeditor/unifieddiffeditorwidget.cpp +++ b/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.addApplyAction(menu, diffFileIndex, chunkIndex); - m_controller.addRevertAction(menu, diffFileIndex, chunkIndex); + m_controller.addCodePasterAction(menu, fileIndex, chunkIndex); + m_controller.addApplyAction(menu, fileIndex, chunkIndex); + m_controller.addRevertAction(menu, fileIndex, chunkIndex); + m_controller.addExtraActions(menu, fileIndex, chunkIndex); } void UnifiedDiffEditorWidget::clear(const QString &message) diff --git a/src/plugins/diffeditor/unifieddiffeditorwidget.h b/src/plugins/diffeditor/unifieddiffeditorwidget.h index d3206a8ee2d..33ca371277f 100644 --- a/src/plugins/diffeditor/unifieddiffeditorwidget.h +++ b/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. diff --git a/src/plugins/git/gitclient.cpp b/src/plugins/git/gitclient.cpp index 5e7ee1e2262..f642f8ba7e6 100644 --- a/src/plugins/git/gitclient.cpp +++ b/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 diffController + = qobject_cast(sender()); + + auto stageChunk = [this](QPointer 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(sender()); - - if (!isValid || !m_contextController) { + if (!diffController || !diffController->chunkExists(fileIndex, chunkIndex)) { stageChunkAction->setEnabled(false); unstageChunkAction->setEnabled(false); } } -void GitClient::slotStageChunk() -{ - 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) +void GitClient::stage(DiffEditor::DiffEditorController *diffController, + 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); diff --git a/src/plugins/git/gitclient.h b/src/plugins/git/gitclient.h index d98d46d4903..206a3c7b7bb 100644 --- a/src/plugins/git/gitclient.h +++ b/src/plugins/git/gitclient.h @@ -329,12 +329,11 @@ public: private: void finishSubmoduleUpdate(); - void slotChunkActionsRequested(QMenu *menu, bool isValid); - void slotStageChunk(); - void slotUnstageChunk(); + void chunkActionsRequested(QMenu *menu, int fileIndex, int chunkIndex); 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 m_stashInfo; QStringList m_updatedSubmodules; bool m_disableEditor; - QPointer m_contextController; QFutureSynchronizer m_synchronizer; // for commit updates };