feat (gitlab): disable comments on unsupported file changes in editor and show a warning

#IJPL-204613 Fixed GitOrigin-RevId: 1a431191ab6f71f6db520a4b0f589836609b9299
2026-10-01 09:03:31 +07:00 · 2025-11-06 00:09:58 +00:00
parent bd259b3cfe
commit 74ccc4c5d5
5 changed files with 105 additions and 34 deletions
@@ -9,6 +9,7 @@ import com.intellij.openapi.actionSystem.DefaultActionGroup
 import com.intellij.openapi.actionSystem.Separator
 import com.intellij.openapi.application.EDT
 import com.intellij.openapi.application.EdtImmediate
+import com.intellij.openapi.application.UI
 import com.intellij.openapi.diff.LineStatusMarkerColorScheme
 import com.intellij.openapi.editor.Document
 import com.intellij.openapi.editor.Editor
@@ -21,6 +22,8 @@ import com.intellij.ui.JBColor
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.awaitCancellation
 import kotlinx.coroutines.withContext
+import org.jetbrains.annotations.ApiStatus
+import org.jetbrains.annotations.Nls
 import java.awt.Color

 object ReviewInEditorUtil {
@@ -105,23 +108,50 @@ object ReviewInEditorUtil {
  }

  suspend fun showReviewToolbarWithActions(vm: CodeReviewInEditorViewModel, editor: Editor, vararg additionalActions: AnAction): Nothing {
-    withContext(Dispatchers.EDT) {
-      val toolbarActionGroup = DefaultActionGroup(
+    val toolbarActionGroup = withContext(Dispatchers.UI) {
+      DefaultActionGroup(
        *additionalActions,
        CodeReviewInEditorToolbarActionGroup(vm),
        Separator.getInstance()
      )
+    }

+    showInspectionWidgetAction(editor, toolbarActionGroup)
+  }
+
+  /**
+   * This is a very special case for GitLab plugin to show on a file with an empty diff
+   */
+  @ApiStatus.Internal
+  suspend fun showReviewToolbarWithWarning(
+    vm: CodeReviewInEditorViewModel, editor: Editor,
+    vararg additionalActions: AnAction,
+    warningSupplier: () -> @Nls String,
+  ): Nothing {
+    val toolbarActionGroup = withContext(Dispatchers.UI) {
+      DefaultActionGroup(
+        *additionalActions,
+        CodeReviewInEditorToolbarActionGroup(vm, warningSupplier),
+        Separator.getInstance()
+      )
+    }
+
+    showInspectionWidgetAction(editor, toolbarActionGroup)
+  }
+
+  // Awaits cancellation indefinitely until scope is cancelled
+  private suspend fun showInspectionWidgetAction(editor: Editor, action: AnAction): Nothing {
+    withContext(Dispatchers.EDT) {
      val editorMarkupModel = editor.markupModel as? EditorMarkupModel
      if (editorMarkupModel == null) {
        error("Editor markup model is not available")
      }
-      editorMarkupModel.addInspectionWidgetAction(toolbarActionGroup, Constraints.FIRST)
+      editorMarkupModel.addInspectionWidgetAction(action, Constraints.FIRST)
      try {
        awaitCancellation()
      }
      finally {
-        editorMarkupModel.removeInspectionWidgetAction(toolbarActionGroup)
+        editorMarkupModel.removeInspectionWidgetAction(action)
      }
    }
  }
@@ -15,10 +15,14 @@ import com.intellij.openapi.project.DumbAware
 import com.intellij.openapi.project.DumbAwareAction
 import com.intellij.openapi.util.NlsActions
 import org.jetbrains.annotations.ApiStatus
+import org.jetbrains.annotations.Nls
 import javax.swing.Icon

@ApiStatus.Internal
-class CodeReviewInEditorToolbarActionGroup(private val vm: CodeReviewInEditorViewModel) : ActionGroup(), DumbAware {
+class CodeReviewInEditorToolbarActionGroup(
+  private val vm: CodeReviewInEditorViewModel,
+  private val customWarningSupplier: (() -> @Nls String)? = null,
+) : ActionGroup(), DumbAware {
  private val updateAction = UpdateAction()

  private val disableReviewAction =
@@ -49,15 +53,17 @@ class CodeReviewInEditorToolbarActionGroup(private val vm: CodeReviewInEditorVie
    val synced = !vm.updateRequired.value
    with(e.presentation) {
      description = CollaborationToolsBundle.message("review.editor.mode.description.title")
+      val customWarning = customWarningSupplier?.invoke()
+      val detailedDescription = customWarning ?: CollaborationToolsBundle.message("review.editor.mode.description")
      val tooltip = HelpTooltip()
        .setTitle(CollaborationToolsBundle.message("review.editor.mode.description.title"))
-        .setDescription(CollaborationToolsBundle.message("review.editor.mode.description"))
+        .setDescription(detailedDescription)
      putClientProperty(ActionButton.CUSTOM_HELP_TOOLTIP, tooltip)

      if (shown) {
        text = CollaborationToolsBundle.message("review.editor.mode.title")
        putClientProperty(ActionUtil.SHOW_TEXT_IN_TOOLBAR, true)
-        icon = if (synced) null else getWarningIcon()
+        icon = if (customWarning != null || !synced) getWarningIcon() else null
      }
      else {
        text = null
@@ -154,7 +154,9 @@ merge.request.review.submit.action.tooltip=Submit merge request review
 merge.request.details.changes.empty=No changes
 merge.request.diff.file.name=Diff for Merge Request !{0}
 merge.request.diff.empty.patch.warning=Comments are disabled for this change because the GitLab server returned an empty diff.\
-  \ Typically, this means that the file is too big. If this is not the case, please report the issue via the Help menu.
+  \ This means that either this file is too big or it was renamed or moved without any changes.
+merge.request.editor.empty.patch.warning=Review Mode is disabled for this file because the GitLab server returned an empty diff.\
+  \ This means that either this file is too big or it was renamed or moved without any changes.

 # Merge request timeline
 merge.request.timeline.error=Failed to load timeline
@@ -29,6 +29,8 @@ import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO
 import org.jetbrains.plugins.gitlab.mergerequest.GitLabMergeRequestsPreferences
 import org.jetbrains.plugins.gitlab.mergerequest.ui.GitLabContextDataLoader
 import org.jetbrains.plugins.gitlab.mergerequest.ui.GitLabProjectViewModel
+import org.jetbrains.plugins.gitlab.mergerequest.ui.editor.GitLabMergeRequestEditorReviewViewModel.FileReviewState
+import org.jetbrains.plugins.gitlab.util.GitLabBundle
 import org.jetbrains.plugins.gitlab.util.GitLabStatistics

@OptIn(ExperimentalCoroutinesApi::class)
@@ -56,28 +58,52 @@ internal class GitLabMergeRequestEditorReviewController(private val project: Pro
        .flatMapLatest {
          it?.currentMergeRequestReviewVm ?: flowOf(null)
        }.collectLatest { reviewVm ->
-          reviewVm?.getFileVm(file)?.collectScoped { fileVm ->
-            if (fileVm != null) supervisorScope {
-              val actionManager = serviceAsync<ActionManager>()
-              launchNow {
-                ReviewInEditorUtil.showReviewToolbarWithActions(
-                  reviewVm, editor,
-                  actionManager.getAction("CodeReview.PreviousComment"),
-                  actionManager.getAction("CodeReview.NextComment"),
-                )
-              }
-
-              val enabledFlow = reviewVm.discussionsViewOption.map { it != DiscussionsViewOption.DONT_SHOW }
-              val syncedFlow = reviewVm.localRepositorySyncStatus.map { it?.getOrNull()?.incoming != true }
-              combine(enabledFlow, syncedFlow) { enabled, synced -> enabled && synced }.distinctUntilChanged().collectLatest { enabled ->
-                if (enabled) showReview(fileVm, editor)
-              }
+          reviewVm?.getFileStateFlow(file)?.collectScoped { fileState ->
+            when (fileState) {
+              is FileReviewState.ReviewEnabled -> showReview(reviewVm, fileState.vm, editor)
+              FileReviewState.ReviewDisabledEmptyDiff -> showEmptyDiffNotification(reviewVm, editor)
+              FileReviewState.NotInReview -> return@collectScoped
            }
          }
        }
    }.cancelOnDispose(editorDisposable)
  }

+  private suspend fun showReview(
+    reviewVm: GitLabMergeRequestEditorReviewViewModel,
+    fileVm: GitLabMergeRequestEditorReviewFileViewModel,
+    editor: EditorEx,
+  ): Nothing {
+    supervisorScope {
+      val actionManager = serviceAsync<ActionManager>()
+      launchNow {
+        ReviewInEditorUtil.showReviewToolbarWithActions(
+          reviewVm, editor,
+          actionManager.getAction("CodeReview.PreviousComment"),
+          actionManager.getAction("CodeReview.NextComment"),
+        )
+      }
+
+      val enabledFlow = reviewVm.discussionsViewOption.map { it != DiscussionsViewOption.DONT_SHOW }
+      val syncedFlow = reviewVm.localRepositorySyncStatus.map { it?.getOrNull()?.incoming != true }
+      combine(enabledFlow, syncedFlow) { enabled, synced -> enabled && synced }.distinctUntilChanged().collectLatest { enabled ->
+        if (enabled) showReview(fileVm, editor)
+      }
+      awaitCancellation()
+    }
+  }
+
+  private suspend fun showEmptyDiffNotification(reviewVm: GitLabMergeRequestEditorReviewViewModel, editor: Editor): Nothing {
+    val actionManager = serviceAsync<ActionManager>()
+    ReviewInEditorUtil.showReviewToolbarWithWarning(
+      reviewVm, editor,
+      actionManager.getAction("CodeReview.PreviousComment"),
+      actionManager.getAction("CodeReview.NextComment")
+    ) {
+      GitLabBundle.message("merge.request.editor.empty.patch.warning")
+    }
+  }
+
  private suspend fun showReview(
    fileVm: GitLabMergeRequestEditorReviewFileViewModel,
    editor: EditorEx,
@@ -123,7 +149,7 @@ internal class GitLabMergeRequestEditorReviewController(private val project: Pro
  private fun CoroutineScope.createRenderer(
    inlayModel: GitLabMergeRequestEditorMappedComponentModel,
    avatarIconsProvider: IconsProvider<GitLabUserDTO>,
-    contextDataLoader: GitLabContextDataLoader
+    contextDataLoader: GitLabContextDataLoader,
  ) =
    when (inlayModel) {
      is GitLabMergeRequestEditorMappedComponentModel.Discussion<*> ->
@@ -81,7 +81,7 @@ class GitLabMergeRequestEditorReviewViewModel internal constructor(
    changes.patchesByChange.filterKeys { it in allChanges }
  }

-  private val filesVms: MutableMap<FilePath, Flow<GitLabMergeRequestEditorReviewFileViewModel?>> = mutableMapOf()
+  private val filesVms: MutableMap<FilePath, Flow<FileReviewState>> = mutableMapOf()
  private val diffRequestsMulticaster = EventDispatcher.create(DiffRequestListener::class.java)

  internal val discussions: StateFlow<ComputedResult<Collection<GitLabMergeRequestEditorDiscussionViewModel>>> =
@@ -211,29 +211,30 @@ class GitLabMergeRequestEditorReviewViewModel internal constructor(
  /**
   * A view model for [virtualFile] review
   */
-  fun getFileVm(virtualFile: VirtualFile): Flow<GitLabMergeRequestEditorReviewFileViewModel?> {
+  fun getFileStateFlow(virtualFile: VirtualFile): Flow<FileReviewState> {
    if (!virtualFile.isValid || virtualFile.isDirectory ||
        !VfsUtilCore.isAncestor(projectMapping.remote.repository.root, virtualFile, true)) {
-      return flowOf(null)
+      return flowOf(FileReviewState.NotInReview)
    }
    val filePath = VcsContextFactory.getInstance().createFilePathOn(virtualFile)
    changesRequest.tryEmit(Unit)
    //TODO: do not recreate VMs on changes change
    return filesVms.getOrPut(filePath) {
-      actualChanges.filterNotNull().map { parsedChanges ->
+      actualChanges.filterNotNull().mapScoped { parsedChanges ->
        val change = parsedChanges.changes.find { it.filePathAfter == filePath }
        if (change == null) {
-          return@map null
+          return@mapScoped FileReviewState.NotInReview
        }
        val diffData = parsedChanges.patchesByChange[change] ?: run {
          LOG.info("Diff data not found for change $change")
-          return@map null
+          return@mapScoped FileReviewState.NotInReview
+        }
+        if (diffData.patch.hunks.isEmpty()) {
+          return@mapScoped FileReviewState.ReviewDisabledEmptyDiff
        }

        val changeSelection = ListSelection.create(parsedChanges.changes, change)
-        changeSelection to diffData
-      }.mapNullableScoped { (changes, diffData) ->
-        createChangeVm(changes, diffData)
+        FileReviewState.ReviewEnabled(createChangeVm(changeSelection, diffData))
      }
    }
  }
@@ -271,6 +272,12 @@ class GitLabMergeRequestEditorReviewViewModel internal constructor(
    data object Error : ChangesState
    class Loaded(val changes: GitBranchComparisonResult) : ChangesState
  }
+
+  sealed interface FileReviewState {
+    data object NotInReview : FileReviewState
+    data object ReviewDisabledEmptyDiff : FileReviewState
+    data class ReviewEnabled(val vm: GitLabMergeRequestEditorReviewFileViewModel) : FileReviewState
+  }
 }

 private fun interface DiffRequestListener : EventListener {