From 66db99aecd78e0a5089f9bc32bca1619f9fbe03c Mon Sep 17 00:00:00 2001 From: Konstantin Kolosovsky Date: Sun, 16 Jun 2019 20:39:01 +0300 Subject: [PATCH] vcs: Allow excluding changed blocks from non-modal commit (IDEA-215860) GitOrigin-RevId: 7a12b6dbe3823693cc14092c3dedc23ca716a5f3 --- .../ui/MultipleLocalChangeListsBrowser.java | 5 +- .../ui/PartiallyExcludedFilesStateHolder.kt | 51 +++++++++++-------- .../vcs/commit/ChangesViewCommitPanel.kt | 11 ++-- .../ChangesViewCommitWorkflowHandler.kt | 7 +++ .../vcs/commit/ChangesViewCommitWorkflowUi.kt | 2 + .../vcs/commit/PartialCommitInclusionModel.kt | 20 ++++---- .../vcs/BasePartiallyExcludedChangesTest.kt | 6 ++- 7 files changed, 64 insertions(+), 38 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/MultipleLocalChangeListsBrowser.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/MultipleLocalChangeListsBrowser.java index 189729e281ca..ba1befa27f10 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/MultipleLocalChangeListsBrowser.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/MultipleLocalChangeListsBrowser.java @@ -51,6 +51,7 @@ import java.util.Optional; import static com.intellij.openapi.util.text.StringUtil.shortenTextWithEllipsis; import static com.intellij.openapi.vcs.changes.ui.ChangesListView.UNVERSIONED_FILES_DATA_KEY; +import static com.intellij.util.containers.ContainerUtil.immutableSingletonList; import static com.intellij.util.ui.update.MergingUpdateQueue.ANY_COMPONENT; class MultipleLocalChangeListsBrowser extends CommitDialogChangesBrowser implements Disposable { @@ -95,7 +96,7 @@ class MultipleLocalChangeListsBrowser extends CommitDialogChangesBrowser impleme } } - myInclusionModel = new PartialCommitInclusionModel(myProject, myChangeList); + myInclusionModel = new PartialCommitInclusionModel(myProject); Disposer.register(this, myInclusionModel); getViewer().setInclusionModel(myInclusionModel); Disposer.register(myInclusionModel, () -> getViewer().setInclusionModel(null)); @@ -225,7 +226,7 @@ class MultipleLocalChangeListsBrowser extends CommitDialogChangesBrowser impleme updateDisplayedChanges(); if (isListChanged && mySelectedListChangeListener != null) mySelectedListChangeListener.run(); - myInclusionModel.setChangeList(myChangeList); + myInclusionModel.setChangeLists(immutableSingletonList(myChangeList)); } @Override diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/PartiallyExcludedFilesStateHolder.kt b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/PartiallyExcludedFilesStateHolder.kt index 53f6ca5a0e8c..a8bb9aadbc8b 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/PartiallyExcludedFilesStateHolder.kt +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/PartiallyExcludedFilesStateHolder.kt @@ -7,18 +7,30 @@ import com.intellij.openapi.vcs.ex.ExclusionState import com.intellij.openapi.vcs.ex.LineStatusTracker import com.intellij.openapi.vcs.ex.PartialLocalLineStatusTracker import com.intellij.openapi.vcs.impl.LineStatusTrackerManager +import com.intellij.util.containers.ContainerUtil.canonicalStrategy import com.intellij.util.ui.update.MergingUpdateQueue import com.intellij.util.ui.update.Update +import gnu.trove.THashMap import gnu.trove.THashSet +import gnu.trove.TObjectHashingStrategy import org.jetbrains.annotations.CalledInAwt -import java.util.* -abstract class PartiallyExcludedFilesStateHolder(project: Project, private var myChangelistId: String) : Disposable { +abstract class PartiallyExcludedFilesStateHolder( + project: Project, + private val hashingStrategy: TObjectHashingStrategy = canonicalStrategy() +) : Disposable { + + private fun PartialLocalLineStatusTracker.setExcludedFromCommit(element: T, isExcluded: Boolean) = + getChangeListId(element)?.let { setExcludedFromCommit(it, isExcluded) } + + private fun PartialLocalLineStatusTracker.getExcludedFromCommitState(element: T): ExclusionState = + getChangeListId(element)?.let { getExcludedFromCommitState(it) } ?: ExclusionState.NO_CHANGES + protected val myUpdateQueue = MergingUpdateQueue(PartiallyExcludedFilesStateHolder::class.java.name, 300, true, MergingUpdateQueue.ANY_COMPONENT, this) - private val myIncludedElements = THashSet() - private val myTrackerExclusionStates = HashMap() + private val myIncludedElements = THashSet(hashingStrategy) + private val myTrackerExclusionStates = THashMap(hashingStrategy) init { MyTrackerManagerListener().install(project) @@ -27,23 +39,19 @@ abstract class PartiallyExcludedFilesStateHolder(project: Project, private va override fun dispose() = Unit protected abstract val trackableElements: Sequence - protected abstract fun findElementFor(tracker: PartialLocalLineStatusTracker): T? + protected abstract fun getChangeListId(element: T): String? + protected abstract fun findElementFor(tracker: PartialLocalLineStatusTracker, changeListId: String): T? protected abstract fun findTrackerFor(element: T): PartialLocalLineStatusTracker? private val trackers get() = trackableElements.mapNotNull { element -> findTrackerFor(element)?.let { tracker -> element to tracker } } - fun setChangelistId(changelistId: String) { - myChangelistId = changelistId - updateExclusionStates() - } - @CalledInAwt open fun updateExclusionStates() { myTrackerExclusionStates.clear() trackers.forEach { (element, tracker) -> - val state = tracker.getExcludedFromCommitState(myChangelistId) + val state = tracker.getExcludedFromCommitState(element) if (state != ExclusionState.NO_CHANGES) myTrackerExclusionStates[element] = state } } @@ -75,17 +83,20 @@ abstract class PartiallyExcludedFilesStateHolder(project: Project, private va override fun onTrackerAdded(tracker: LineStatusTracker<*>) { if (tracker !is PartialLocalLineStatusTracker) return - findElementFor(tracker)?.let { element -> tracker.setExcludedFromCommit(element !in myIncludedElements) } + tracker.getAffectedChangeListsIds().forEach { changeListId -> + findElementFor(tracker, changeListId)?.let { element -> tracker.setExcludedFromCommit(element, element !in myIncludedElements) } + } tracker.addListener(trackerListener, disposable) } override fun onTrackerRemoved(tracker: LineStatusTracker<*>) { if (tracker !is PartialLocalLineStatusTracker) return - findElementFor(tracker)?.let { element -> - myTrackerExclusionStates -= element + tracker.getAffectedChangeListsIds().forEach { changeListId -> + val element = findElementFor(tracker, changeListId) ?: return@forEach - val exclusionState = tracker.getExcludedFromCommitState(myChangelistId) + myTrackerExclusionStates -= element + val exclusionState = tracker.getExcludedFromCommitState(element) if (exclusionState != ExclusionState.NO_CHANGES) { if (exclusionState != ExclusionState.ALL_EXCLUDED) myIncludedElements += element else myIncludedElements -= element } @@ -96,7 +107,7 @@ abstract class PartiallyExcludedFilesStateHolder(project: Project, private va } fun getIncludedSet(): Set { - val set = HashSet(myIncludedElements) + val set = THashSet(myIncludedElements, hashingStrategy) myTrackerExclusionStates.forEach { (element, state) -> if (state == ExclusionState.ALL_EXCLUDED) set -= element else set += element } @@ -104,9 +115,9 @@ abstract class PartiallyExcludedFilesStateHolder(project: Project, private va } fun setIncludedElements(elements: Collection) { - val set = HashSet(elements) + val set = THashSet(elements, hashingStrategy) trackers.forEach { (element, tracker) -> - tracker.setExcludedFromCommit(element !in set) + tracker.setExcludedFromCommit(element, element !in set) } myIncludedElements.clear() @@ -116,14 +127,14 @@ abstract class PartiallyExcludedFilesStateHolder(project: Project, private va } fun includeElements(elements: Collection) { - elements.forEach { findTrackerFor(it)?.setExcludedFromCommit(false) } + elements.forEach { findTrackerFor(it)?.setExcludedFromCommit(it, false) } myIncludedElements += elements updateExclusionStates() } fun excludeElements(elements: Collection) { - elements.forEach { findTrackerFor(it)?.setExcludedFromCommit(true) } + elements.forEach { findTrackerFor(it)?.setExcludedFromCommit(it, true) } myIncludedElements -= elements updateExclusionStates() diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt index 7b9ed4e9c68c..939e9ed61719 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt @@ -14,9 +14,9 @@ import com.intellij.openapi.ui.popup.LightweightWindowEvent import com.intellij.openapi.util.Disposer import com.intellij.openapi.vcs.VcsBundle.message import com.intellij.openapi.vcs.changes.Change -import com.intellij.openapi.vcs.changes.ChangeListChange import com.intellij.openapi.vcs.changes.ChangesViewManager import com.intellij.openapi.vcs.changes.InclusionListener +import com.intellij.openapi.vcs.changes.InclusionModel import com.intellij.openapi.vcs.changes.ui.* import com.intellij.openapi.vcs.changes.ui.ChangesBrowserNode.UNVERSIONED_FILES_TAG import com.intellij.openapi.vcs.changes.ui.VcsTreeModelData.* @@ -112,7 +112,6 @@ class ChangesViewCommitPanel(private val changesView: ChangesListView, private v buildLayout() with(changesView) { - setInclusionModel(DefaultInclusionModel(ChangeListChange.HASHING_STRATEGY)) setInclusionListener { inclusionEventDispatcher.multicaster.inclusionChanged() } isShowCheckboxes = true } @@ -224,6 +223,12 @@ class ChangesViewCommitPanel(private val changesView: ChangesListView, private v override fun getIncludedUnversionedFiles(): List = includedUnderTag(changesView, UNVERSIONED_FILES_TAG).userObjects(VirtualFile::class.java) + override var inclusionModel: InclusionModel? + get() = changesView.inclusionModel + set(value) { + changesView.setInclusionModel(value) + } + override fun isInclusionEmpty(): Boolean = changesView.isInclusionEmpty override fun getInclusion(): Set = changesView.includedSet override fun clearInclusion() = changesView.clearInclusion() @@ -247,8 +252,6 @@ class ChangesViewCommitPanel(private val changesView: ChangesListView, private v with(changesView) { isShowCheckboxes = false setInclusionListener(null) - clearInclusion() - setInclusionModel(null) } } diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt index 2203253b6a4e..0ae5298a5b22 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt @@ -31,8 +31,11 @@ class ChangesViewCommitWorkflowHandler( private val changeListManager = ChangeListManager.getInstance(project) private var knownActiveChanges: Collection = emptyList() + private val inclusionModel = PartialCommitInclusionModel(project) + init { Disposer.register(this, Disposable { workflow.disposeCommitOptions() }) + Disposer.register(this, inclusionModel) Disposer.register(ui, this) workflow.addListener(this, this) @@ -40,6 +43,8 @@ class ChangesViewCommitWorkflowHandler( ui.addExecutorListener(this, this) ui.addDataProvider(createDataProvider()) ui.addInclusionListener(this, this) + ui.inclusionModel = inclusionModel + Disposer.register(inclusionModel, Disposable { ui.inclusionModel = null }) vcsesChanged() // as currently vcses are set before handler subscribes to corresponding event } @@ -89,6 +94,8 @@ class ChangesViewCommitWorkflowHandler( val activeChanges = changeListManager.defaultChangeList.changes knownActiveChanges = knownActiveChanges.intersect(activeChanges) } + + inclusionModel.changeLists = changeLists } fun setCommitState(items: Collection, force: Boolean) { diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt index 630fbf7b7bda..56c0786231fb 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt @@ -3,11 +3,13 @@ package com.intellij.vcs.commit import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.DataContext +import com.intellij.openapi.vcs.changes.InclusionModel interface ChangesViewCommitWorkflowUi : CommitWorkflowUi { var isDefaultCommitActionEnabled: Boolean fun setCustomCommitActions(actions: List) + var inclusionModel: InclusionModel? // TODO Looks better to create "interface ItemInclusionModel" to which ChangesTree will delegate // And just pass such model to CommitWorkflowUi instead of adding include-related methods to CommitWorkflowUi directly fun isInclusionEmpty(): Boolean diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/PartialCommitInclusionModel.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/PartialCommitInclusionModel.kt index 6035965a9a30..4b53848f1e69 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/PartialCommitInclusionModel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/PartialCommitInclusionModel.kt @@ -5,6 +5,7 @@ import com.intellij.openapi.Disposable import com.intellij.openapi.project.Project import com.intellij.openapi.util.Disposer import com.intellij.openapi.vcs.changes.Change +import com.intellij.openapi.vcs.changes.ChangeListChange import com.intellij.openapi.vcs.changes.LocalChangeList import com.intellij.openapi.vcs.changes.ui.BaseInclusionModel import com.intellij.openapi.vcs.changes.ui.PartiallyExcludedFilesStateHolder @@ -14,15 +15,12 @@ import com.intellij.openapi.vcs.impl.PartialChangesUtil.convertExclusionState import com.intellij.openapi.vcs.impl.PartialChangesUtil.getPartialTracker import com.intellij.util.ui.ThreeStateCheckBox -class PartialCommitInclusionModel( - private val project: Project, - changeList: LocalChangeList -) : BaseInclusionModel(), Disposable { +class PartialCommitInclusionModel(private val project: Project) : BaseInclusionModel(), Disposable { - var changeList: LocalChangeList = changeList + var changeLists: Collection = emptyList() set(value) { field = value - stateHolder.setChangelistId(value.id) + stateHolder.updateExclusionStates() } private val stateHolder = StateHolder() @@ -49,11 +47,13 @@ class PartialCommitInclusionModel( override fun dispose() = Unit - private inner class StateHolder : PartiallyExcludedFilesStateHolder(project, changeList.id) { - override val trackableElements: Sequence get() = changeList.changes.asSequence() + private inner class StateHolder : PartiallyExcludedFilesStateHolder(project, ChangeListChange.HASHING_STRATEGY) { + override val trackableElements: Sequence get() = changeLists.asSequence().flatMap { it.changes.asSequence() } - override fun findElementFor(tracker: PartialLocalLineStatusTracker): Any? = - changeList.changes.find { tracker.virtualFile == PartialChangesUtil.getVirtualFile(it) } + override fun getChangeListId(element: Any) = (element as? ChangeListChange)?.changeListId + + override fun findElementFor(tracker: PartialLocalLineStatusTracker, changeListId: String): Any? = + changeLists.find { it.id == changeListId }?.changes?.find { tracker.virtualFile == PartialChangesUtil.getVirtualFile(it) } override fun findTrackerFor(element: Any): PartialLocalLineStatusTracker? = (element as? Change)?.let { getPartialTracker(project, it) } diff --git a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BasePartiallyExcludedChangesTest.kt b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BasePartiallyExcludedChangesTest.kt index e03ca104984f..7451c489977e 100644 --- a/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BasePartiallyExcludedChangesTest.kt +++ b/platform/vcs-tests/testSrc/com/intellij/openapi/vcs/BasePartiallyExcludedChangesTest.kt @@ -16,7 +16,7 @@ abstract class BasePartiallyExcludedChangesTest : BaseLineStatusTrackerManagerTe stateHolder.updateExclusionStates() } - protected inner class MyStateHolder : PartiallyExcludedFilesStateHolder(getProject(), DEFAULT.asListNameToId()) { + protected inner class MyStateHolder : PartiallyExcludedFilesStateHolder(getProject()) { val paths = HashSet() init { @@ -26,7 +26,9 @@ abstract class BasePartiallyExcludedChangesTest : BaseLineStatusTrackerManagerTe override val trackableElements: Sequence get() = paths.asSequence() - override fun findElementFor(tracker: PartialLocalLineStatusTracker): FilePath? { + override fun getChangeListId(element: FilePath): String = DEFAULT.asListNameToId() + + override fun findElementFor(tracker: PartialLocalLineStatusTracker, changeListId: String): FilePath? { return paths.find { it.virtualFile == tracker.virtualFile } }