vcs: Run before commit checks on non-modal commit

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/AbstractCommitWorkflow.kt

@@ -2,18 +2,17 @@
 package com.intellij.openapi.vcs.changes.ui
 
 import com.intellij.openapi.Disposable
+import com.intellij.openapi.diagnostic.logger
 import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.vcs.AbstractVcs
 import com.intellij.openapi.vcs.CheckinProjectPanel
 import com.intellij.openapi.vcs.ProjectLevelVcsManager
-import com.intellij.openapi.vcs.changes.Change
-import com.intellij.openapi.vcs.changes.CommitContext
-import com.intellij.openapi.vcs.changes.LocalChangeList
-import com.intellij.openapi.vcs.changes.PseudoMap
+import com.intellij.openapi.vcs.changes.*
 import com.intellij.openapi.vcs.changes.actions.ScheduleForAdditionAction.addUnversionedFilesToVcs
 import com.intellij.openapi.vcs.checkin.BaseCheckinHandlerFactory
 import com.intellij.openapi.vcs.checkin.CheckinHandler
+import com.intellij.openapi.vcs.checkin.CheckinMetaHandler
 import com.intellij.openapi.vcs.impl.CheckinHandlersManager
 import com.intellij.openapi.vfs.VirtualFile
 import com.intellij.util.EventDispatcher
@@ -23,6 +22,8 @@ import com.intellij.util.containers.ContainerUtil.newUnmodifiableList
 import com.intellij.util.containers.ContainerUtil.unmodifiableOrEmptySet
 import java.util.*
 
+private val LOG = logger<AbstractCommitWorkflow>()
+
 internal fun CommitOptions.saveState() = allOptions.forEach { it.saveState() }
 internal fun CommitOptions.restoreState() = allOptions.forEach { it.restoreState() }
 internal fun CommitOptions.refresh() = allOptions.forEach { it.refresh() }
@@ -85,6 +86,58 @@ abstract class AbstractCommitWorkflow(val project: Project) {
     return addUnversionedFilesToVcs(project, changeList, unversionedFiles, callback, null)
   }
 
+  fun executeDefault(executor: CommitExecutor?) {
+    val beforeCommitChecksResult = runBeforeCommitChecksWithEvents(true, executor)
+    processExecuteDefaultChecksResult(beforeCommitChecksResult)
+  }
+
+  protected open fun processExecuteDefaultChecksResult(result: CheckinHandler.ReturnResult) = Unit
+
+  protected fun runBeforeCommitChecksWithEvents(isDefaultCommit: Boolean, executor: CommitExecutor?): CheckinHandler.ReturnResult {
+    eventDispatcher.multicaster.beforeCommitChecksStarted()
+    val result = runBeforeCommitChecks(executor)
+    eventDispatcher.multicaster.beforeCommitChecksEnded(isDefaultCommit, result)
+
+    return result
+  }
+
+  private fun runBeforeCommitChecks(executor: CommitExecutor?): CheckinHandler.ReturnResult {
+    var result: CheckinHandler.ReturnResult? = null
+    val checks = Runnable {
+      FileDocumentManager.getInstance().saveAllDocuments()
+      result = runBeforeCommitHandlersChecks(executor)
+    }
+
+    doRunBeforeCommitChecks(wrapWithCommitMetaHandlers(checks))
+
+    return result ?: CheckinHandler.ReturnResult.CANCEL
+  }
+
+  protected open fun doRunBeforeCommitChecks(checks: Runnable) = checks.run()
+
+  private fun wrapWithCommitMetaHandlers(block: Runnable): Runnable {
+    var result = block
+    commitHandlers.filterIsInstance<CheckinMetaHandler>().forEach { metaHandler ->
+      val previousResult = result
+      result = Runnable {
+        LOG.debug("CheckinMetaHandler.runCheckinHandlers: $metaHandler")
+        metaHandler.runCheckinHandlers(previousResult)
+      }
+    }
+    return result
+  }
+
+  private fun runBeforeCommitHandlersChecks(executor: CommitExecutor?): CheckinHandler.ReturnResult {
+    commitHandlers.asSequence().filter { it.acceptExecutor(executor) }.forEach { handler ->
+      LOG.debug("CheckinHandler.beforeCheckin: $handler")
+
+      val result = handler.beforeCheckin(executor, additionalDataConsumer)
+      if (result != CheckinHandler.ReturnResult.COMMIT) return result
+    }
+
+    return CheckinHandler.ReturnResult.COMMIT
+  }
+
   companion object {
     @JvmStatic
     fun getCommitHandlerFactories(project: Project): List<BaseCheckinHandlerFactory> =

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/AbstractCommitWorkflowHandler.kt

@@ -97,22 +97,46 @@ abstract class AbstractCommitWorkflowHandler<W : AbstractCommitWorkflow, U : Com
     if (!saveCommitOptions()) return
     saveCommitMessage(true)
 
-    refreshChanges { doExecuteDefault(executor) }
+    refreshChanges {
+      updateWorkflow()
+      doExecuteDefault(executor)
+    }
   }
 
-  protected open fun executeCustom(executor: CommitExecutor, session: CommitSession) = Unit
+  private fun executeCustom(executor: CommitExecutor, session: CommitSession) {
+    if (!canExecute(executor)) return
+    if (!checkEmptyCommitMessage()) return
+    if (!saveCommitOptions()) return
+    saveCommitMessage(true)
+
+    (session as? CommitSessionContextAware)?.setContext(workflow.commitContext)
+    refreshChanges {
+      updateWorkflow()
+      doExecuteCustom(executor, session)
+    }
+  }
+
+  protected open fun updateWorkflow() = Unit
 
   protected abstract fun addUnversionedFiles(): Boolean
 
   protected fun addUnversionedFiles(changeList: LocalChangeList): Boolean =
     workflow.addUnversionedFiles(changeList, getIncludedUnversionedFiles()) { changes -> ui.includeIntoCommit(changes) }
 
-  protected abstract fun doExecuteDefault(executor: CommitExecutor?)
+  private fun doExecuteDefault(executor: CommitExecutor?) = try {
+    workflow.executeDefault(executor)
+  }
+  catch (e: InputException) { // TODO Looks like this catch is unnecessary - check
+    e.show()
+  }
+
+  protected abstract fun canExecute(executor: CommitExecutor): Boolean
+  protected abstract fun doExecuteCustom(executor: CommitExecutor, session: CommitSession)
 
-  protected fun checkEmptyCommitMessage(): Boolean =
+  private fun checkEmptyCommitMessage(): Boolean =
     getCommitMessage().isNotEmpty() || !vcsConfiguration.FORCE_NON_EMPTY_COMMENT || ui.confirmCommitWithEmptyMessage()
 
-  protected fun saveCommitOptions() = try {
+  private fun saveCommitOptions() = try {
     commitOptions.saveState()
     true
   }

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/ChangesViewCommitWorkflowHandler.kt

@@ -6,6 +6,7 @@ import com.intellij.openapi.vcs.CheckinProjectPanel
 import com.intellij.openapi.vcs.VcsConfiguration
 import com.intellij.openapi.vcs.changes.ChangeListManager
 import com.intellij.openapi.vcs.changes.CommitExecutor
+import com.intellij.openapi.vcs.changes.CommitSession
 
 class ChangesViewCommitWorkflowHandler(
   override val workflow: ChangesViewCommitWorkflow,
@@ -39,7 +40,8 @@ class ChangesViewCommitWorkflowHandler(
 
   override fun addUnversionedFiles(): Boolean = addUnversionedFiles(changeListManager.defaultChangeList)
 
-  override fun doExecuteDefault(executor: CommitExecutor?) = Unit
+  override fun canExecute(executor: CommitExecutor): Boolean = false
+  override fun doExecuteCustom(executor: CommitExecutor, session: CommitSession) = Unit
 
   override fun saveCommitMessage(success: Boolean) = VcsConfiguration.getInstance(project).saveCommitMessage(getCommitMessage())
 

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/SingleChangeListCommitWorkflow.kt

@@ -3,7 +3,6 @@ package com.intellij.openapi.vcs.changes.ui
 
 import com.intellij.CommonBundle.getCancelButtonText
 import com.intellij.openapi.diagnostic.logger
-import com.intellij.openapi.fileEditor.FileDocumentManager
 import com.intellij.openapi.progress.ProgressManager
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.ui.Messages
@@ -17,7 +16,6 @@ import com.intellij.openapi.vcs.changes.ui.CommitChangeListDialog.DIALOG_TITLE
 import com.intellij.openapi.vcs.changes.ui.SingleChangeListCommitter.Companion.moveToFailedList
 import com.intellij.openapi.vcs.checkin.CheckinChangeListSpecificComponent
 import com.intellij.openapi.vcs.checkin.CheckinHandler
-import com.intellij.openapi.vcs.checkin.CheckinMetaHandler
 import com.intellij.openapi.vcs.impl.PartialChangesUtil
 import com.intellij.openapi.vcs.impl.PartialChangesUtil.getPartialTracker
 import com.intellij.util.ui.UIUtil.removeMnemonic
@@ -57,20 +55,19 @@ open class SingleChangeListCommitWorkflow(
 
   val commitMessagePolicy: SingleChangeListCommitMessagePolicy = SingleChangeListCommitMessagePolicy(project, initialCommitMessage)
 
-  fun executeDefault(executor: CommitExecutor?, commitState: ChangeListCommitState) {
-    val beforeCommitChecksResult = runBeforeCommitChecksWithEvents(true, executor, commitState.changeList)
-    when (beforeCommitChecksResult) {
-      CheckinHandler.ReturnResult.COMMIT -> DefaultNameChangeListCleaner(project, commitState).use { doCommit(commitState) }
-      CheckinHandler.ReturnResult.CLOSE_WINDOW ->
-        moveToFailedList(project, commitState, message("commit.dialog.rejected.commit.template", commitState.changeList.name))
-      CheckinHandler.ReturnResult.CANCEL -> Unit
-    }
+  internal lateinit var commitState: ChangeListCommitState
+
+  override fun processExecuteDefaultChecksResult(result: CheckinHandler.ReturnResult) = when (result) {
+    CheckinHandler.ReturnResult.COMMIT -> DefaultNameChangeListCleaner(project, commitState).use { doCommit(commitState) }
+    CheckinHandler.ReturnResult.CLOSE_WINDOW ->
+      moveToFailedList(project, commitState, message("commit.dialog.rejected.commit.template", commitState.changeList.name))
+    CheckinHandler.ReturnResult.CANCEL -> Unit
   }
 
   fun executeCustom(executor: CommitExecutor, session: CommitSession, commitState: ChangeListCommitState) {
     if (!configureCommitSession(executor, session, commitState.changes, commitState.commitMessage)) return
 
-    val beforeCommitChecksResult = runBeforeCommitChecksWithEvents(false, executor, commitState.changeList)
+    val beforeCommitChecksResult = runBeforeCommitChecksWithEvents(false, executor)
     when (beforeCommitChecksResult) {
       CheckinHandler.ReturnResult.COMMIT -> {
         val success = doCommitCustom(executor, session, commitState)
@@ -96,53 +93,8 @@ open class SingleChangeListCommitWorkflow(
     }
   }
 
-  private fun runBeforeCommitChecksWithEvents(isDefaultCommit: Boolean,
-                                              executor: CommitExecutor?,
-                                              changeList: LocalChangeList): CheckinHandler.ReturnResult {
-    eventDispatcher.multicaster.beforeCommitChecksStarted()
-    val result = runBeforeCommitChecks(executor, changeList)
-    eventDispatcher.multicaster.beforeCommitChecksEnded(isDefaultCommit, result)
-
-    return result
-  }
-
-  private fun runBeforeCommitChecks(executor: CommitExecutor?, changeList: LocalChangeList): CheckinHandler.ReturnResult {
-    var result: CheckinHandler.ReturnResult? = null
-    val checks = Runnable {
-      FileDocumentManager.getInstance().saveAllDocuments()
-      result = runBeforeCommitHandlersChecks(executor)
-    }
-
-    doRunBeforeCommitChecks(changeList, wrapWithCommitMetaHandlers(checks))
-
-    return result ?: CheckinHandler.ReturnResult.CANCEL
-  }
-
-  protected open fun doRunBeforeCommitChecks(changeList: LocalChangeList, checks: Runnable) =
-    PartialChangesUtil.runUnderChangeList(project, changeList, checks)
-
-  private fun wrapWithCommitMetaHandlers(block: Runnable): Runnable {
-    var result = block
-    commitHandlers.filterIsInstance<CheckinMetaHandler>().forEach { metaHandler ->
-      val previousResult = result
-      result = Runnable {
-        LOG.debug("CheckinMetaHandler.runCheckinHandlers: $metaHandler")
-        metaHandler.runCheckinHandlers(previousResult)
-      }
-    }
-    return result
-  }
-
-  private fun runBeforeCommitHandlersChecks(executor: CommitExecutor?): CheckinHandler.ReturnResult {
-    commitHandlers.asSequence().filter { it.acceptExecutor(executor) }.forEach { handler ->
-      LOG.debug("CheckinHandler.beforeCheckin: $handler")
-
-      val result = handler.beforeCheckin(executor, additionalDataConsumer)
-      if (result != CheckinHandler.ReturnResult.COMMIT) return result
-    }
-
-    return CheckinHandler.ReturnResult.COMMIT
-  }
+  override fun doRunBeforeCommitChecks(checks: Runnable) =
+    PartialChangesUtil.runUnderChangeList(project, commitState.changeList, checks)
 
   open fun canExecute(executor: CommitExecutor, changes: Collection<Change>): Boolean {
     if (!executor.supportsPartialCommit()) {

platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ui/SingleChangeListCommitWorkflowHandler.kt

 // Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.openapi.vcs.changes.ui
 
-import com.intellij.openapi.ui.InputException
 import com.intellij.openapi.util.Disposer
 import com.intellij.openapi.vcs.CheckinProjectPanel
-import com.intellij.openapi.vcs.changes.*
+import com.intellij.openapi.vcs.changes.ChangeListManagerImpl
 import com.intellij.openapi.vcs.changes.ChangesUtil.getAffectedVcses
 import com.intellij.openapi.vcs.changes.ChangesUtil.getAffectedVcsesForFiles
+import com.intellij.openapi.vcs.changes.CommitExecutor
+import com.intellij.openapi.vcs.changes.CommitExecutorBase
+import com.intellij.openapi.vcs.changes.CommitSession
 import com.intellij.openapi.vcs.checkin.CheckinHandler
 import com.intellij.openapi.vcs.impl.LineStatusTrackerManager
 
@@ -79,24 +81,16 @@ class SingleChangeListCommitWorkflowHandler(
 
   override fun customCommitSucceeded() = ui.deactivate()
 
-  override fun addUnversionedFiles() = addUnversionedFiles(getChangeList())
-
-  override fun doExecuteDefault(executor: CommitExecutor?) = try {
-    workflow.executeDefault(executor, getCommitState())
-  }
-  catch (e: InputException) { // TODO Looks like this catch is unnecessary - check
-    e.show()
+  override fun updateWorkflow() {
+    workflow.commitState = getCommitState()
   }
 
-  override fun executeCustom(executor: CommitExecutor, session: CommitSession) {
-    if (!workflow.canExecute(executor, getIncludedChanges())) return
-    if (!checkEmptyCommitMessage()) return
-    if (!saveCommitOptions()) return
-    saveCommitMessage(true)
+  override fun addUnversionedFiles() = addUnversionedFiles(getChangeList())
 
-    (session as? CommitSessionContextAware)?.setContext(workflow.commitContext)
-    refreshChanges { workflow.executeCustom(executor, session, getCommitState()) }
-  }
+  override fun canExecute(executor: CommitExecutor): Boolean = workflow.canExecute(executor, getIncludedChanges())
+
+  override fun doExecuteCustom(executor: CommitExecutor, session: CommitSession) =
+    workflow.executeCustom(executor, session, getCommitState())
 
   private fun initCommitMessage() {
     commitMessagePolicy.init(getChangeList(), getIncludedChanges())

plugins/svn4idea/src/org/jetbrains/idea/svn/integrate/AlienCommitWorkflow.kt

@@ -4,7 +4,6 @@ package org.jetbrains.idea.svn.integrate
 import com.intellij.openapi.vcs.AbstractVcs
 import com.intellij.openapi.vcs.changes.Change
 import com.intellij.openapi.vcs.changes.CommitExecutor
-import com.intellij.openapi.vcs.changes.LocalChangeList
 import com.intellij.openapi.vcs.changes.ui.ChangeListCommitState
 import com.intellij.openapi.vcs.changes.ui.CommitChangeListDialog.DIALOG_TITLE
 import com.intellij.openapi.vcs.changes.ui.DefaultCommitResultHandler
@@ -14,7 +13,7 @@ class AlienCommitWorkflow(val vcs: AbstractVcs<*>, changeListName: String, chang
   SingleChangeListCommitWorkflow(vcs.project, changes, vcsToCommit = vcs, initialCommitMessage = commitMessage) {
   val changeList = AlienLocalChangeList(changes, changeListName)
 
-  override fun doRunBeforeCommitChecks(changeList: LocalChangeList, checks: Runnable) = checks.run()
+  override fun doRunBeforeCommitChecks(checks: Runnable) = checks.run()
 
   override fun canExecute(executor: CommitExecutor, changes: Collection<Change>) = true