From cc5483abbcd4a1875c48f433fce6284e4431154d Mon Sep 17 00:00:00 2001 From: Ivan Semenov Date: Tue, 4 Nov 2025 19:11:12 +0100 Subject: [PATCH] feat (gitlab): disable comments on unsupported file changes in editor and show a warning #IJPL-204613 Fixed IJ-CR-181587 (cherry picked from commit 1a431191ab6f71f6db520a4b0f589836609b9299) # Conflicts: # community/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewController.kt # community/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewViewModel.kt (cherry picked from commit 6cf315fce34fd715e81364571228cc80c5cdb7a2) IJ-MR-183241 GitOrigin-RevId: a81cae6dda265885f62f5a4b0c4754850341c747 --- .../codereview/editor/ReviewInEditorUtil.kt | 38 ++++++++++++++-- .../CodeReviewInEditorToolbarActionGroup.kt | 12 ++++-- .../messages/GitLabBundle.properties | 4 +- ...itLabMergeRequestEditorReviewController.kt | 43 ++++++++++++++----- ...GitLabMergeRequestEditorReviewViewModel.kt | 40 ++++++++++------- 5 files changed, 102 insertions(+), 35 deletions(-) diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/ReviewInEditorUtil.kt b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/ReviewInEditorUtil.kt index 3a172d9713c6..f1d8b105c00c 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/ReviewInEditorUtil.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/ReviewInEditorUtil.kt @@ -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) } } } diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/action/CodeReviewInEditorToolbarActionGroup.kt b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/action/CodeReviewInEditorToolbarActionGroup.kt index 0acafbb3c9c6..53ea37a59f4b 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/action/CodeReviewInEditorToolbarActionGroup.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/action/CodeReviewInEditorToolbarActionGroup.kt @@ -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 diff --git a/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties b/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties index 1726b727bb73..47af1dd94fc6 100644 --- a/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties +++ b/plugins/gitlab/gitlab-core/resources/messages/GitLabBundle.properties @@ -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 diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewController.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewController.kt index a31b1e15a84f..98ea648a1c90 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewController.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewController.kt @@ -25,6 +25,8 @@ import kotlinx.coroutines.flow.* import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO import org.jetbrains.plugins.gitlab.mergerequest.GitLabMergeRequestsPreferences 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) @@ -52,23 +54,42 @@ internal class GitLabMergeRequestEditorReviewController(private val project: Pro .flatMapLatest { it?.currentMergeRequestReviewVm ?: flowOf(null) }.collectLatest { reviewVm -> - reviewVm?.getFileVm(file)?.collectScoped { fileVm -> - if (fileVm != null) supervisorScope { - launchNow { - ReviewInEditorUtil.showReviewToolbar(reviewVm, editor) - } - - 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 { + launchNow { + ReviewInEditorUtil.showReviewToolbar(reviewVm, editor) + } + + 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 { + ReviewInEditorUtil.showReviewToolbarWithWarning(reviewVm, editor) { + GitLabBundle.message("merge.request.editor.empty.patch.warning") + } + } + private suspend fun showReview(fileVm: GitLabMergeRequestEditorReviewFileViewModel, editor: EditorEx): Nothing { withContext(Dispatchers.Main) { val preferences = project.serviceAsync() diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewViewModel.kt index 27bbe622db78..035ef75dbc7d 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewViewModel.kt @@ -3,7 +3,9 @@ package org.jetbrains.plugins.gitlab.mergerequest.ui.editor import com.intellij.collaboration.async.launchNow import com.intellij.collaboration.async.mapNullableScoped +import com.intellij.collaboration.async.mapScoped import com.intellij.collaboration.async.mapState +import com.intellij.collaboration.async.stateInNow import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.DiscussionsViewOption import com.intellij.collaboration.ui.codereview.editor.CodeReviewInEditorViewModel @@ -67,8 +69,13 @@ class GitLabMergeRequestEditorReviewViewModel internal constructor( private val changesRequest = MutableSharedFlow(replay = 1) val actualChangesState: StateFlow = _actualChangesState.asStateFlow() + private val actualChanges: StateFlow = actualChangesState.mapNotNull { + (it as? ChangesState.Loaded)?.changes + }.distinctUntilChangedBy { + it.baseSha + it.headSha + it.mergeBaseSha + }.stateInNow(cs, null) - private val filesVms: MutableMap> = mutableMapOf() + private val filesVms: MutableMap> = mutableMapOf() private val diffRequestsMulticaster = EventDispatcher.create(DiffRequestListener::class.java) @OptIn(ExperimentalCoroutinesApi::class) @@ -140,34 +147,29 @@ class GitLabMergeRequestEditorReviewViewModel internal constructor( /** * A view model for [virtualFile] review */ - fun getFileVm(virtualFile: VirtualFile): Flow { + fun getFileStateFlow(virtualFile: VirtualFile): Flow { 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) { - actualChangesState.mapNotNull { - (it as? ChangesState.Loaded)?.changes - }.distinctUntilChangedBy { - it.baseSha + it.headSha + it.mergeBaseSha - }.transform { parsedChanges -> - val change = parsedChanges.changes.find { it.filePathAfter == filePath } + actualChanges.mapScoped { parsedChanges -> + val change = parsedChanges?.changes?.find { it.filePathAfter == filePath } if (change == null) { - emit(null) - return@transform + return@mapScoped FileReviewState.NotInReview } val diffData = parsedChanges.patchesByChange[change] ?: run { LOG.info("Diff data not found for change $change") - emit(null) - return@transform + return@mapScoped FileReviewState.NotInReview + } + if (diffData.patch.hunks.isEmpty()) { + return@mapScoped FileReviewState.ReviewDisabledEmptyDiff } val changeSelection = ListSelection.create(parsedChanges.changes, change) - emit(changeSelection to diffData) - }.mapNullableScoped { (change, diffData) -> - createChangeVm(change, diffData) + FileReviewState.ReviewEnabled(createChangeVm(changeSelection, diffData)) } } } @@ -203,6 +205,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 {