From aeece045c918c1bd4fb282cf0e16628fbefee860 Mon Sep 17 00:00:00 2001 From: "Ilia.Shulgin" Date: Fri, 26 Sep 2025 13:22:05 +0200 Subject: [PATCH] [vcs] IJPL-173924 Don't expose inclusionModel in BackendCommitChangesViewModel The only usage of getter was `ChangeListViewCommitProgressPanel#inclusionChanged`, which is redundant as inclusion notifier ensures that it was actually changed. Also, replace `setInclusionListener` with SharedFlow and get rid of `ChangeListViewCommitProgressPanel` GitOrigin-RevId: df80b2bdff0f6b051ff615769ee39d00cda44eba --- platform/vcs-impl/api-dump.txt | 6 ++-- .../BackendCommitChangesViewModel.kt | 6 ++-- .../BackendLocalCommitChangesViewModel.kt | 18 +++++------ .../BackendRemoteCommitChangesViewModel.kt | 11 +++++-- .../vcs/commit/ChangeListViewCommitPanel.kt | 32 +++---------------- .../vcs/commit/ChangesViewCommitPanel.kt | 24 +++++++------- .../ChangesViewCommitWorkflowHandler.kt | 4 +-- .../vcs/commit/ChangesViewCommitWorkflowUi.kt | 2 +- .../vcs/commit/CommitProgressPanel.kt | 4 +++ 9 files changed, 47 insertions(+), 60 deletions(-) diff --git a/platform/vcs-impl/api-dump.txt b/platform/vcs-impl/api-dump.txt index 083b314282f1..98ecf2e9b29d 100644 --- a/platform/vcs-impl/api-dump.txt +++ b/platform/vcs-impl/api-dump.txt @@ -3948,12 +3948,11 @@ a:com.intellij.vcs.commit.ChangeListViewCommitPanel - f:getEditedCommit():com.intellij.platform.vcs.impl.shared.commit.EditedCommitPresentation - f:getIncludedChanges():java.util.List - f:getIncludedUnversionedFiles():java.util.List -- f:getInclusionModel():com.intellij.openapi.vcs.changes.InclusionModel - f:select(java.lang.Object):V - f:selectFirst(java.util.Collection):V - f:setCompletionContext(java.util.List):V - f:setEditedCommit(com.intellij.platform.vcs.impl.shared.commit.EditedCommitPresentation):V -- f:setInclusionModel(com.intellij.openapi.vcs.changes.InclusionModel):V +- setInclusionModel(com.intellij.openapi.vcs.changes.InclusionModel):V f:com.intellij.vcs.commit.ChangesViewCommitPanel - com.intellij.vcs.commit.NonModalCommitPanel - com.intellij.vcs.commit.ChangesViewCommitWorkflowUi @@ -3968,7 +3967,6 @@ f:com.intellij.vcs.commit.ChangesViewCommitPanel - getEditedCommit():com.intellij.platform.vcs.impl.shared.commit.EditedCommitPresentation - getIncludedChanges():java.util.List - getIncludedUnversionedFiles():java.util.List -- getInclusionModel():com.intellij.openapi.vcs.changes.InclusionModel - isActive():Z - refreshChangesViewBeforeCommit(kotlin.coroutines.Continuation):java.lang.Object - select(java.lang.Object):V @@ -3981,7 +3979,6 @@ com.intellij.vcs.commit.ChangesViewCommitWorkflowUi - a:deactivate(Z):V - a:endExecution():V - a:expand(java.lang.Object):V -- a:getInclusionModel():com.intellij.openapi.vcs.changes.InclusionModel - a:isActive():Z - a:refreshChangesViewBeforeCommit(kotlin.coroutines.Continuation):java.lang.Object - a:select(java.lang.Object):V @@ -4088,6 +4085,7 @@ c:com.intellij.vcs.commit.CommitProgressPanel - com.intellij.openapi.vcs.changes.InclusionListener - com.intellij.vcs.commit.CommitProgressUi - (com.intellij.openapi.project.Project):V +- (com.intellij.openapi.project.Project,com.intellij.vcs.commit.CommitWorkflowUi,com.intellij.ui.EditorTextComponent):V - addCommitCheckFailure(com.intellij.vcs.commit.CommitCheckFailure):V - p:buildErrorText():java.lang.String - clearCommitCheckFailures():V diff --git a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendCommitChangesViewModel.kt b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendCommitChangesViewModel.kt index 72135d7d4201..d21aff6e5480 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendCommitChangesViewModel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendCommitChangesViewModel.kt @@ -6,11 +6,14 @@ import com.intellij.openapi.vcs.FilePath import com.intellij.openapi.vcs.changes.ui.ChangesListView import com.intellij.openapi.vfs.VirtualFile import com.intellij.vcs.commit.ChangesViewCommitWorkflowHandler +import kotlinx.coroutines.flow.SharedFlow import org.jetbrains.annotations.ApiStatus import javax.swing.JComponent // TODO IJPL-173924 cleanup methods returning tree/component internal interface BackendCommitChangesViewModel { + val inclusionChanged: SharedFlow + fun initPanel() fun setCommitWorkflowHandler(handler: ChangesViewCommitWorkflowHandler?) @@ -24,8 +27,7 @@ internal interface BackendCommitChangesViewModel { fun setGrouping(groupingKey: String) fun resetViewImmediatelyAndRefreshLater() - fun setInclusionListener(listener: Runnable?) - var inclusionModel: InclusionModel? + fun setInclusionModel(model: InclusionModel?) fun setShowCheckboxes(value: Boolean) fun getDisplayedChanges(): List diff --git a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendLocalCommitChangesViewModel.kt b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendLocalCommitChangesViewModel.kt index 4d258a573be0..c46e1fa53bc4 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendLocalCommitChangesViewModel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendLocalCommitChangesViewModel.kt @@ -14,20 +14,24 @@ import com.intellij.platform.vcs.impl.shared.changes.ChangesViewSettings import com.intellij.util.ui.tree.TreeUtil import com.intellij.util.ui.tree.TreeUtil.* import com.intellij.vcs.commit.ChangesViewCommitWorkflowHandler +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.asSharedFlow import javax.swing.JComponent import javax.swing.tree.TreePath internal class BackendLocalCommitChangesViewModel(private val panel: CommitChangesViewWithToolbarPanel) : BackendCommitChangesViewModel { private var commitWorkflowHandler: ChangesViewCommitWorkflowHandler? = null + private val _inclusionChanged = MutableSharedFlow() - override var inclusionModel: InclusionModel? - get() = panel.changesView.inclusionModel - set(value) { - panel.changesView.setInclusionModel(value) - } + override val inclusionChanged = _inclusionChanged.asSharedFlow() + + override fun setInclusionModel(model: InclusionModel?) { + panel.changesView.setInclusionModel(model) + } override fun initPanel() { panel.initPanel(ModelProvider()) + panel.changesView.setInclusionListener { _inclusionChanged.tryEmit(Unit) } } override fun setCommitWorkflowHandler(handler: ChangesViewCommitWorkflowHandler?) { @@ -58,10 +62,6 @@ internal class BackendLocalCommitChangesViewModel(private val panel: CommitChang panel.resetViewImmediatelyAndRefreshLater() } - override fun setInclusionListener(listener: Runnable?) { - panel.changesView.setInclusionListener(listener) - } - override fun setShowCheckboxes(value: Boolean) { panel.changesView.isShowCheckboxes = value } diff --git a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendRemoteCommitChangesViewModel.kt b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendRemoteCommitChangesViewModel.kt index 024bd8fa7c88..76ea446397c4 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendRemoteCommitChangesViewModel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/changes/viewModel/BackendRemoteCommitChangesViewModel.kt @@ -11,14 +11,21 @@ import com.intellij.openapi.vcs.changes.LocalChangesListView import com.intellij.openapi.vcs.changes.ui.ChangesListView import com.intellij.openapi.vfs.VirtualFile import com.intellij.vcs.commit.ChangesViewCommitWorkflowHandler +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import javax.swing.JComponent // TODO IJPL-173924 Propper RPC-based implementation internal class BackendRemoteCommitChangesViewModel(private val project: Project) : BackendCommitChangesViewModel { private var horizontal: Boolean = true private val treeView: ChangesListView by lazy { LocalChangesListView(project) } + override val inclusionChanged = MutableSharedFlow() - override var inclusionModel: InclusionModel? = null + private val inclusionModel = MutableStateFlow(null) + + override fun setInclusionModel(model: InclusionModel?) { + inclusionModel.value = model + } override fun initPanel() { } @@ -46,8 +53,6 @@ internal class BackendRemoteCommitChangesViewModel(private val project: Project) override fun resetViewImmediatelyAndRefreshLater() { } - override fun setInclusionListener(listener: Runnable?) {} - override fun setShowCheckboxes(value: Boolean) {} override fun getDisplayedChanges(): List = emptyList() diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangeListViewCommitPanel.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangeListViewCommitPanel.kt index a8c78e117c21..37c9050947dd 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangeListViewCommitPanel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangeListViewCommitPanel.kt @@ -16,9 +16,7 @@ import com.intellij.openapi.vcs.changes.ui.ChangesListView import com.intellij.openapi.vcs.changes.ui.EditChangelistSupport import com.intellij.openapi.vcs.changes.ui.VcsTreeModelData.* import com.intellij.platform.vcs.impl.shared.commit.EditedCommitPresentation -import com.intellij.ui.EditorTextComponent import com.intellij.util.application -import com.intellij.util.ui.JBUI import com.intellij.util.ui.UIUtil import com.intellij.util.ui.tree.TreeUtil.* import org.jetbrains.annotations.ApiStatus @@ -29,7 +27,7 @@ abstract class ChangeListViewCommitPanel @ApiStatus.Internal constructor( project: Project, private val changesView: ChangesListView, ) : NonModalCommitPanel(project), ChangesViewCommitWorkflowUi { - private val progressPanel = ChangeListViewCommitProgressPanel(project, this, commitMessage.editorField) + private val progressPanel = CommitProgressPanel(project, this, commitMessage.editorField) private val commitActions = commitActionsPanel.createActions() private var rootComponent: JComponent? = null @@ -98,11 +96,9 @@ abstract class ChangeListViewCommitPanel @ApiStatus.Internal constructor( final override fun getIncludedUnversionedFiles(): List = includedUnderTag(changesView, UNVERSIONED_FILES_TAG).userObjects(FilePath::class.java) - final override var inclusionModel: InclusionModel? - get() = changesView.inclusionModel - set(value) { - changesView.setInclusionModel(value) - } + override fun setInclusionModel(model: InclusionModel?) { + changesView.setInclusionModel(model) + } final override val commitProgressUi: CommitProgressUi get() = progressPanel @@ -122,23 +118,3 @@ abstract class ChangeListViewCommitPanel @ApiStatus.Internal constructor( changesView.setInclusionListener(null) } } - -internal class ChangeListViewCommitProgressPanel( - project: Project, - private val commitWorkflowUi: ChangesViewCommitWorkflowUi, - commitMessage: EditorTextComponent, -) : CommitProgressPanel(project) { - - private var oldInclusion: Set = emptySet() - - init { - setup(commitWorkflowUi, commitMessage, JBUI.Borders.empty()) - } - - override fun inclusionChanged() { - val newInclusion = commitWorkflowUi.inclusionModel?.getInclusion().orEmpty() - - if (oldInclusion != newInclusion) super.inclusionChanged() - oldInclusion = newInclusion - } -} 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 14a9c74702c7..63e606fdf05c 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitPanel.kt @@ -2,7 +2,7 @@ package com.intellij.vcs.commit import com.intellij.openapi.Disposable -import com.intellij.openapi.application.WriteIntentReadAction +import com.intellij.openapi.application.writeIntentReadAction import com.intellij.openapi.diagnostic.logger import com.intellij.openapi.project.Project import com.intellij.openapi.util.Disposer @@ -16,11 +16,16 @@ import com.intellij.openapi.vcs.changes.ui.* import com.intellij.openapi.vcs.changes.ui.ChangesViewContentManager.Companion.LOCAL_CHANGES import com.intellij.openapi.vcs.changes.ui.ChangesViewContentManager.Companion.getToolWindowFor import com.intellij.openapi.wm.ToolWindow +import com.intellij.platform.util.coroutines.childScope import com.intellij.platform.vcs.impl.shared.commit.EditedCommitPresentation import com.intellij.util.application import com.intellij.util.ui.UIUtil +import com.intellij.vcs.VcsDisposable import com.intellij.vcs.changes.BackendChangesView import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import org.jetbrains.annotations.ApiStatus import javax.swing.JComponent import kotlin.properties.Delegates.observable @@ -31,7 +36,7 @@ class ChangesViewCommitPanel internal constructor( ) : NonModalCommitPanel(project), ChangesViewCommitWorkflowUi { private var isHideToolWindowOnCommit = false - private val progressPanel = ChangeListViewCommitProgressPanel(project, this, commitMessage.editorField) + private val progressPanel = CommitProgressPanel(project, this, commitMessage.editorField) private val commitActions = commitActionsPanel.createActions() private var rootComponent: JComponent? = null @@ -47,9 +52,9 @@ class ChangesViewCommitPanel internal constructor( support.installSearch(commitMessage.editorField, commitMessage.editorField) } - changesView.viewModel.setInclusionListener { - WriteIntentReadAction.run { fireInclusionChanged() } - } + val scope = VcsDisposable.getInstance(project).coroutineScope.childScope("ChangesViewCommitPanel") + Disposer.register(this) { scope.cancel() } + changesView.viewModel.inclusionChanged.onEach { writeIntentReadAction { fireInclusionChanged() } }.launchIn(scope) commitActionsPanel.isCommitButtonDefault = { !progressPanel.isDumbMode && UIUtil.isFocusAncestor(rootComponent ?: component) @@ -98,11 +103,9 @@ class ChangesViewCommitPanel internal constructor( override fun getIncludedUnversionedFiles(): List = changesView.viewModel.getIncludedUnversionedFiles() - override var inclusionModel: InclusionModel? - get() = changesView.viewModel.inclusionModel - set(value) { - changesView.viewModel.inclusionModel = value - } + override fun setInclusionModel(model: InclusionModel?) { + changesView.viewModel.setInclusionModel(model) + } override val commitProgressUi: CommitProgressUi get() = progressPanel @@ -120,7 +123,6 @@ class ChangesViewCommitPanel internal constructor( override fun dispose() { super.dispose() changesView.viewModel.setShowCheckboxes(false) - changesView.viewModel.setInclusionListener(null) } override fun activate(): Boolean { 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 749470195150..505844f4911b 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowHandler.kt @@ -66,8 +66,8 @@ class ChangesViewCommitWorkflowHandler( ui.addExecutorListener(this, this) ui.addDataProvider(EdtNoGetDataProvider { sink -> uiDataSnapshot(sink) }) ui.addInclusionListener(this, this) - ui.inclusionModel = inclusionModel - Disposer.register(inclusionModel, Disposable { ui.inclusionModel = null }) + ui.setInclusionModel(inclusionModel) + Disposer.register(inclusionModel, Disposable { ui.setInclusionModel(null) }) ui.setCompletionContext(changeListManager.changeLists) setupDumbModeTracking() 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 eb432cc8da88..b83bc3b31ee6 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/ChangesViewCommitWorkflowUi.kt @@ -12,7 +12,7 @@ interface ChangesViewCommitWorkflowUi : NonModalCommitWorkflowUi { suspend fun refreshChangesViewBeforeCommit() - var inclusionModel: InclusionModel? + fun setInclusionModel(model: InclusionModel?) fun expand(item: Any) fun select(item: Any) diff --git a/platform/vcs-impl/src/com/intellij/vcs/commit/CommitProgressPanel.kt b/platform/vcs-impl/src/com/intellij/vcs/commit/CommitProgressPanel.kt index 983e36a642d1..eed7e9de4fe0 100644 --- a/platform/vcs-impl/src/com/intellij/vcs/commit/CommitProgressPanel.kt +++ b/platform/vcs-impl/src/com/intellij/vcs/commit/CommitProgressPanel.kt @@ -81,6 +81,10 @@ private fun JBLabel.setWarning(@NlsContexts.Label warningText: String) { open class CommitProgressPanel(project: Project) : CommitProgressUi, InclusionListener, DocumentListener, Disposable { private val scope = VcsDisposable.getInstance(project).coroutineScope.childScope("CommitProgressPanel", Dispatchers.EDT) + constructor(project: Project, commitWorkflowUi: CommitWorkflowUi, commitMessage: EditorTextComponent) : this(project) { + setup(commitWorkflowUi, commitMessage, empty()) + } + private val taskInfo = CommitChecksTaskInfo() private val progressFlow = MutableStateFlow(null) private var progress: InlineCommitChecksProgressIndicator? by progressFlow::value