[gh] Add next/previous comment navigation to in-editor review (IJPL-180555)

#IJPL-180555 Fixed

GitOrigin-RevId: 615bd13324607b4eaa7c6f145f2e233095dcc3db

platform/collaboration-tools/api-dump-experimental.txt

@@ -1049,6 +1049,7 @@ f:com.intellij.collaboration.ui.codereview.diff.viewer.DiffViewerUtilKt
 - f:getREVIEW_CHANGES_STATUS_COLOR():com.intellij.ui.JBColor
 - f:getREVIEW_STATUS_MARKER_COLOR_SCHEME():com.intellij.openapi.diff.LineStatusMarkerColorScheme
 - f:showReviewToolbar(com.intellij.collaboration.ui.codereview.editor.CodeReviewInEditorViewModel,com.intellij.openapi.editor.Editor,kotlin.coroutines.Continuation):java.lang.Object
+- f:showReviewToolbarWithActions(com.intellij.collaboration.ui.codereview.editor.CodeReviewInEditorViewModel,com.intellij.openapi.editor.Editor,com.intellij.openapi.actionSystem.AnAction[],kotlin.coroutines.Continuation):java.lang.Object
 - f:trackDocumentDiffSync(com.intellij.openapi.editor.Document,com.intellij.openapi.editor.Document,kotlin.jvm.functions.Function1,kotlin.coroutines.Continuation):java.lang.Object
 - f:trackDocumentDiffSync(java.lang.CharSequence,com.intellij.openapi.editor.Document,kotlin.jvm.functions.Function1,kotlin.coroutines.Continuation):java.lang.Object
 - f:transferLineFromAfter(java.util.List,I,Z):java.lang.Integer

platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/comment/CodeReviewCommentUIUtil.kt

@@ -49,12 +49,13 @@ import javax.swing.*
 
 object CodeReviewCommentUIUtil {
 
-  const val INLAY_PADDING = 10
+  const val INLAY_PADDING: Int = 10
   private const val EDITOR_INLAY_PANEL_ARC = 10
 
-  val COMMENT_BUBBLE_BORDER_COLOR: Color = JBColor.namedColor("Review.ChatItem.BubblePanel.Border",
-                                                              JBColor.namedColor("EditorTabs.underTabsBorderColor",
-                                                                          JBColor.border()))
+  val COMMENT_BUBBLE_BORDER_COLOR: Color = JBColor.namedColor(
+    "Review.ChatItem.BubblePanel.Border",
+    JBColor.namedColor("EditorTabs.underTabsBorderColor", JBColor.border())
+  )
 
   fun getInlayPadding(componentType: CodeReviewChatItemUIUtil.ComponentType): Insets {
     val paddingInsets = componentType.paddingInsets
@@ -100,14 +101,22 @@ object CodeReviewCommentUIUtil {
     return roundedPanel
   }
 
+  /**
+   * We suppress the outer editor context because this will cause registered editor actions not to be usable.
+   * This means they don't steal key events from this component.
+   * Specifically; the TAB key tends to escape the focused component if Editor is registered.
+   */
   private fun suppressOuterEditorData(sink: DataSink) {
-    arrayOf(CommonDataKeys.EDITOR, CommonDataKeys.HOST_EDITOR, CommonDataKeys.EDITOR_EVEN_IF_INACTIVE,
-            CommonDataKeys.CARET,
-            CommonDataKeys.VIRTUAL_FILE, CommonDataKeys.VIRTUAL_FILE_ARRAY,
-            CommonDataKeys.LANGUAGE,
-            CommonDataKeys.PSI_FILE, CommonDataKeys.PSI_ELEMENT,
-            PlatformCoreDataKeys.FILE_EDITOR,
-            PlatformCoreDataKeys.PSI_ELEMENT_ARRAY).forEach {
+    arrayOf(
+      CommonDataKeys.EDITOR,
+      CommonDataKeys.HOST_EDITOR,
+      CommonDataKeys.CARET,
+      CommonDataKeys.VIRTUAL_FILE, CommonDataKeys.VIRTUAL_FILE_ARRAY,
+      CommonDataKeys.LANGUAGE,
+      CommonDataKeys.PSI_FILE, CommonDataKeys.PSI_ELEMENT,
+      PlatformCoreDataKeys.FILE_EDITOR,
+      PlatformCoreDataKeys.PSI_ELEMENT_ARRAY
+    ).forEach {
       sink.setNull(it)
     }
   }
@@ -152,9 +161,11 @@ object CodeReviewCommentUIUtil {
     return button
   }
 
-  fun createFoldedThreadControlsIn(cs: CoroutineScope,
-                                   vm: CodeReviewFoldableThreadViewModel,
-                                   avatarIconsProvider: IconsProvider<CodeReviewUser>): JComponent {
+  fun createFoldedThreadControlsIn(
+    cs: CoroutineScope,
+    vm: CodeReviewFoldableThreadViewModel,
+    avatarIconsProvider: IconsProvider<CodeReviewUser>,
+  ): JComponent {
     val authorsLabel = JLabel().apply {
       bindVisibilityIn(cs, vm.repliesState.map { it.repliesCount > 0 })
       bindIconIn(cs, vm.repliesState.map {

platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/ReviewInEditorUtil.kt

@@ -3,6 +3,7 @@ package com.intellij.collaboration.ui.codereview.editor
 
 import com.intellij.collaboration.ui.codereview.editor.action.CodeReviewInEditorToolbarActionGroup
 import com.intellij.diff.util.Range
+import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.Constraints
 import com.intellij.openapi.actionSystem.DefaultActionGroup
 import com.intellij.openapi.actionSystem.Separator
@@ -100,8 +101,13 @@ object ReviewInEditorUtil {
    * @throws IllegalStateException when the actions were not set up
    */
   suspend fun showReviewToolbar(vm: CodeReviewInEditorViewModel, editor: Editor): Nothing {
+    showReviewToolbarWithActions(vm, editor)
+  }
+
+  suspend fun showReviewToolbarWithActions(vm: CodeReviewInEditorViewModel, editor: Editor, vararg additionalActions: AnAction): Nothing {
     withContext(Dispatchers.EDT) {
       val toolbarActionGroup = DefaultActionGroup(
+        *additionalActions,
         CodeReviewInEditorToolbarActionGroup(vm),
         Separator.getInstance()
       )

plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/comment/action/GHPRDiffReviewNextCommentAction.kt

@@ -6,6 +6,7 @@ import com.intellij.diff.tools.util.DiffDataKeys
 import com.intellij.openapi.actionSystem.ActionUpdateThread
 import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
+import com.intellij.openapi.actionSystem.CommonDataKeys
 import org.jetbrains.plugins.github.pullrequest.ui.diff.GHPRReviewDiffEditorModel
 
 internal class GHPRDiffReviewNextCommentAction : AnAction() {
@@ -14,7 +15,7 @@ internal class GHPRDiffReviewNextCommentAction : AnAction() {
   override fun update(e: AnActionEvent) {
     val project = e.project ?: return
 
-    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR)
+    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR) ?: e.getData(CommonDataKeys.EDITOR_EVEN_IF_INACTIVE)
 
     val editorModel = editor?.getUserData(CodeReviewNavigableEditorViewModel.KEY)
                       ?: editor?.getUserData(GHPRReviewDiffEditorModel.KEY)
@@ -34,7 +35,7 @@ internal class GHPRDiffReviewNextCommentAction : AnAction() {
   override fun actionPerformed(e: AnActionEvent) {
     val project = e.project ?: return
 
-    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR)
+    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR) ?: e.getData(CommonDataKeys.EDITOR_EVEN_IF_INACTIVE)
     val editorModel = editor?.getUserData(CodeReviewNavigableEditorViewModel.KEY)
                       ?: editor?.getUserData(GHPRReviewDiffEditorModel.KEY)
     if (editor == null || editorModel == null) return

plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/comment/action/GHPRDiffReviewPreviousCommentAction.kt

@@ -6,6 +6,7 @@ import com.intellij.diff.tools.util.DiffDataKeys
 import com.intellij.openapi.actionSystem.ActionUpdateThread
 import com.intellij.openapi.actionSystem.AnAction
 import com.intellij.openapi.actionSystem.AnActionEvent
+import com.intellij.openapi.actionSystem.CommonDataKeys
 import org.jetbrains.plugins.github.pullrequest.ui.diff.GHPRReviewDiffEditorModel
 
 internal class GHPRDiffReviewPreviousCommentAction : AnAction() {
@@ -14,7 +15,7 @@ internal class GHPRDiffReviewPreviousCommentAction : AnAction() {
   override fun update(e: AnActionEvent) {
     val project = e.project ?: return
 
-    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR)
+    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR) ?: e.getData(CommonDataKeys.EDITOR_EVEN_IF_INACTIVE)
 
     val editorModel = editor?.getUserData(CodeReviewNavigableEditorViewModel.KEY)
                       ?: editor?.getUserData(GHPRReviewDiffEditorModel.KEY)
@@ -34,7 +35,7 @@ internal class GHPRDiffReviewPreviousCommentAction : AnAction() {
   override fun actionPerformed(e: AnActionEvent) {
     val project = e.project ?: return
 
-    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR)
+    val editor = e.getData(DiffDataKeys.CURRENT_EDITOR) ?: e.getData(CommonDataKeys.EDITOR_EVEN_IF_INACTIVE)
     val editorModel = editor?.getUserData(CodeReviewNavigableEditorViewModel.KEY)
                       ?: editor?.getUserData(GHPRReviewDiffEditorModel.KEY)
     if (editor == null || editorModel == null) return

plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/GHPRReviewFileEditorModel.kt

@@ -8,12 +8,14 @@ import com.intellij.collaboration.async.stateInNow
 import com.intellij.collaboration.ui.codereview.editor.*
 import com.intellij.collaboration.util.ExcludingApproximateChangedRangesShifter
 import com.intellij.collaboration.util.Hideable
+import com.intellij.collaboration.util.RefComparisonChange
 import com.intellij.collaboration.util.syncOrToggleAll
 import com.intellij.diff.util.LineRange
 import com.intellij.diff.util.Range
 import com.intellij.openapi.Disposable
 import com.intellij.openapi.util.Key
 import com.intellij.util.cancelOnDispose
+import com.intellij.util.concurrency.annotations.RequiresEdt
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.StateFlow
@@ -25,10 +27,12 @@ internal class GHPRReviewFileEditorModel internal constructor(
   private val cs: CoroutineScope,
   private val settings: GithubPullRequestsProjectUISettings,
   private val fileVm: GHPRReviewFileEditorViewModel,
-  private val changesModel: MutableCodeReviewEditorGutterChangesModel = MutableCodeReviewEditorGutterChangesModel()
+  private val changesModel: MutableCodeReviewEditorGutterChangesModel = MutableCodeReviewEditorGutterChangesModel(),
+  @RequiresEdt private val showEditor: (RefComparisonChange, Int) -> Unit
 ) : CodeReviewEditorGutterChangesModel by changesModel,
     CodeReviewEditorGutterActionableChangesModel,
-    CodeReviewEditorModel<GHPREditorMappedComponentModel> {
+    CodeReviewEditorModel<GHPREditorMappedComponentModel>,
+    CodeReviewNavigableEditorViewModel {
 
   private val postReviewRanges = MutableStateFlow<List<Range>?>(null)
 
@@ -96,6 +100,49 @@ internal class GHPRReviewFileEditorModel internal constructor(
     changesModel.setChanges(ExcludingApproximateChangedRangesShifter.shift(fileVm.changedRanges, changedRanges).map(Range::asLst))
   }
 
+  override fun canGotoNextComment(focusedThreadId: String): Boolean = fileVm.lookupNextComment(focusedThreadId) != null
+  override fun canGotoNextComment(line: Int): Boolean = fileVm.lookupNextComment(line.shiftLineToBefore()) != null
+  override fun canGotoPreviousComment(focusedThreadId: String): Boolean = fileVm.lookupPreviousComment(focusedThreadId) != null
+  override fun canGotoPreviousComment(line: Int): Boolean = fileVm.lookupPreviousComment(line.shiftLineToBefore()) != null
+
+  @RequiresEdt
+  override fun gotoNextComment(focusedThreadId: String) {
+    val commentId = fileVm.lookupNextComment(focusedThreadId) ?: return
+    gotoComment(commentId)
+  }
+
+  @RequiresEdt
+  override fun gotoNextComment(line: Int) {
+    val commentId = fileVm.lookupNextComment(line.shiftLineToBefore()) ?: return
+    gotoComment(commentId)
+  }
+
+  @RequiresEdt
+  override fun gotoPreviousComment(focusedThreadId: String) {
+    val commentId = fileVm.lookupPreviousComment(focusedThreadId) ?: return
+    gotoComment(commentId)
+  }
+
+  @RequiresEdt
+  override fun gotoPreviousComment(line: Int) {
+    val commentId = fileVm.lookupPreviousComment(line.shiftLineToBefore()) ?: return
+    gotoComment(commentId)
+  }
+
+  @RequiresEdt
+  private fun gotoComment(threadId: String) {
+    val (change, unmappedLine) = fileVm.getThreadPosition(threadId) ?: return
+    val line = if (change == fileVm.change) {
+      // Only shift the line if it comes from this file
+      unmappedLine.shiftLineToAfter()
+      // if the line number is from a different file, we can't currently easily access outside changes to shift with
+      // the current line would be a best-guess estimate
+    } else unmappedLine
+
+    showEditor(change, line)
+    fileVm.requestThreadFocus(threadId)
+  }
+
   private fun StateFlow<Int?>.shiftLine(): StateFlow<Int?> =
     combineState(postReviewRanges) { line, ranges ->
       if (ranges != null && line != null) {
@@ -104,6 +151,16 @@ internal class GHPRReviewFileEditorModel internal constructor(
       else null
     }
 
+  private fun Int.shiftLineToAfter(): Int {
+    val ranges = postReviewRanges.value ?: return this
+    return ReviewInEditorUtil.transferLineToAfter(ranges, this)
+  }
+
+  private fun Int.shiftLineToBefore(): Int {
+    val ranges = postReviewRanges.value ?: return this
+    return ReviewInEditorUtil.transferLineFromAfter(ranges, this, approximate = true) ?: 0
+  }
+
   private inner class ShiftedThread(vm: GHPRReviewFileEditorThreadViewModel)
     : GHPREditorMappedComponentModel.Thread<GHPRReviewFileEditorThreadViewModel>(vm) {
     override val isVisible: StateFlow<Boolean> = vm.isVisible.combineState(hiddenState) { visible, hidden -> visible && !hidden }

plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/GHPRReviewFileEditorViewModel.kt

@@ -27,10 +27,13 @@ import org.jetbrains.plugins.github.pullrequest.data.GHPRDataContext
 import org.jetbrains.plugins.github.pullrequest.data.provider.GHPRDataProvider
 import org.jetbrains.plugins.github.pullrequest.ui.comment.GHPRReviewCommentLocation
 import org.jetbrains.plugins.github.pullrequest.ui.comment.GHPRReviewCommentPosition
+import org.jetbrains.plugins.github.pullrequest.ui.comment.GHPRReviewUnifiedPosition
 import org.jetbrains.plugins.github.pullrequest.ui.comment.GHPRThreadsViewModels
 import org.jetbrains.plugins.github.ui.avatars.GHAvatarIconsProvider
 
 interface GHPRReviewFileEditorViewModel {
+  val change: RefComparisonChange
+
   val iconProvider: GHAvatarIconsProvider
   val currentUser: GHUser
 
@@ -47,6 +50,14 @@ interface GHPRReviewFileEditorViewModel {
 
   val linesWithComments: StateFlow<Set<Int>>
 
+  fun lookupNextComment(focusedThreadId: String): String?
+  fun lookupNextComment(line: Int): String?
+  fun lookupPreviousComment(focusedThreadId: String): String?
+  fun lookupPreviousComment(line: Int): String?
+
+  fun getThreadPosition(threadId: String): Pair<RefComparisonChange, Int>?
+  fun requestThreadFocus(threadId: String)
+
   fun requestNewComment(lineIdx: Int, focus: Boolean)
   fun cancelNewComment(lineIdx: Int)
 
@@ -60,7 +71,7 @@ internal class GHPRReviewFileEditorViewModelImpl(
   parentCs: CoroutineScope,
   dataContext: GHPRDataContext,
   dataProvider: GHPRDataProvider,
-  private val change: RefComparisonChange,
+  override val change: RefComparisonChange,
   private val diffData: GitTextFilePatchWithHistory,
   private val threadsVm: GHPRThreadsViewModels,
   private val allMappedThreads: StateFlow<Map<String, MappedGHPRReviewEditorThreadViewModel.MappingData>>,
@@ -124,6 +135,37 @@ internal class GHPRReviewFileEditorViewModelImpl(
     }
   }
 
+  override fun lookupNextComment(focusedThreadId: String): String? =
+    threadsVm.lookupNextComment(focusedThreadId, this::threadIsVisible)
+
+  override fun lookupNextComment(line: Int): String? =
+    threadsVm.lookupNextComment(lineToUnified(line), this::threadIsVisible)
+
+  override fun lookupPreviousComment(focusedThreadId: String): String? =
+    threadsVm.lookupPreviousComment(focusedThreadId, this::threadIsVisible)
+
+  override fun lookupPreviousComment(line: Int): String? =
+    threadsVm.lookupPreviousComment(lineToUnified(line), this::threadIsVisible)
+
+  override fun getThreadPosition(threadId: String): Pair<RefComparisonChange, Int>? {
+    val position = allMappedThreads.value[threadId] ?: return null
+    return (position.change ?: return null) to (position.line ?: return null)
+  }
+
+  override fun requestThreadFocus(threadId: String) {
+    threadsVm.compactThreads.value.find { it.id == threadId }?.requestFocus()
+  }
+
+  /**
+   * We don't really care about the left-sided line number. It needs to be at the beginning to make sure
+   * the first comment on the line is picked though.
+   */
+  private fun lineToUnified(line: Int): GHPRReviewUnifiedPosition =
+    GHPRReviewUnifiedPosition(change, leftLine = -1, rightLine = line)
+
+  private fun threadIsVisible(threadId: String): Boolean =
+    allMappedThreads.value[threadId]?.let { it.isVisible && it.line != null && it.change?.filePathAfter != null } ?: false
+
   override fun requestNewComment(lineIdx: Int, focus: Boolean) {
     val position = GHPRReviewCommentPosition(change, GHPRReviewCommentLocation.SingleLine(Side.RIGHT, lineIdx))
     val sharedVm = threadsVm.requestNewComment(position)

plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/GHPRReviewInEditorController.kt

@@ -8,6 +8,7 @@ import com.intellij.collaboration.ui.codereview.diff.DiscussionsViewOption
 import com.intellij.collaboration.ui.codereview.editor.*
 import com.intellij.collaboration.util.HashingUtil
 import com.intellij.collaboration.util.getOrNull
+import com.intellij.openapi.actionSystem.ActionManager
 import com.intellij.openapi.components.Service
 import com.intellij.openapi.components.service
 import com.intellij.openapi.components.serviceAsync
@@ -17,6 +18,8 @@ import com.intellij.openapi.editor.event.EditorFactoryEvent
 import com.intellij.openapi.editor.event.EditorFactoryListener
 import com.intellij.openapi.editor.ex.EditorEx
 import com.intellij.openapi.editor.ex.util.EditorUtil
+import com.intellij.openapi.fileEditor.FileEditorManager
+import com.intellij.openapi.fileEditor.OpenFileDescriptor
 import com.intellij.openapi.project.Project
 import com.intellij.openapi.util.Disposer
 import com.intellij.util.cancelOnDispose
@@ -40,6 +43,8 @@ internal class GHPRReviewInEditorController(private val project: Project, privat
     if (!isPotentialEditor(editor)) return
     val file = editor.virtualFile ?: return
 
+    val actionManager = ActionManager.getInstance()
+
     val editorDisposable = Disposer.newDisposable().also {
       EditorUtil.disposeWithEditor(editor, it)
     }
@@ -56,13 +61,17 @@ internal class GHPRReviewInEditorController(private val project: Project, privat
           reviewVm?.getViewModelFor(file)?.collectScoped { fileVm ->
             if (fileVm != null) supervisorScope {
               launchNow {
-                ReviewInEditorUtil.showReviewToolbar(reviewVm, editor)
+                ReviewInEditorUtil.showReviewToolbarWithActions(
+                  reviewVm, editor,
+                  actionManager.getAction("GitHub.Diff.Review.PreviousComment"),
+                  actionManager.getAction("GitHub.Diff.Review.NextComment"),
+                )
               }
 
               val enabledFlow = reviewVm.discussionsViewOption.map { it != DiscussionsViewOption.DONT_SHOW }
               val syncedFlow = reviewVm.updateRequired.map { !it }
               combine(enabledFlow, syncedFlow) { enabled, synced -> enabled && synced }.distinctUntilChanged().collectLatest { enabled ->
-                if (enabled) showReview(settings, fileVm, editor)
+                if (enabled) showReview(project, settings, fileVm, editor)
               }
             }
           }
@@ -75,11 +84,16 @@ internal class GHPRReviewInEditorController(private val project: Project, privat
   }
 }
 
-private suspend fun showReview(settings: GithubPullRequestsProjectUISettings, fileVm: GHPRReviewFileEditorViewModel, editor: EditorEx): Nothing {
+private suspend fun showReview(project: Project, settings: GithubPullRequestsProjectUISettings, fileVm: GHPRReviewFileEditorViewModel, editor: EditorEx): Nothing {
   withContext(Dispatchers.Main.immediate) {
     val reviewHeadContent = fileVm.originalContent.mapNotNull { it?.result?.getOrThrow() }.first()
 
-    val model = GHPRReviewFileEditorModel(this, settings, fileVm)
+    val model = GHPRReviewFileEditorModel(this, settings, fileVm) showEditor@{ changeToShow, lineIdx ->
+      val file = changeToShow.filePathAfter?.virtualFile ?: return@showEditor
+      val fileOpenDescriptor = OpenFileDescriptor(project, file, lineIdx, 0)
+      FileEditorManager.getInstance(project).openFileEditor(fileOpenDescriptor, true)
+    }
+
     launchNow {
       ReviewInEditorUtil.trackDocumentDiffSync(reviewHeadContent, editor.document, model::setPostReviewChanges)
     }
@@ -94,12 +108,15 @@ private suspend fun showReview(settings: GithubPullRequestsProjectUISettings, fi
       val userIcon = fileVm.iconProvider.getIcon(fileVm.currentUser.url, 16)
       editor.renderInlays(model.inlays, HashingUtil.mappingStrategy(GHPREditorMappedComponentModel::key)) { createRenderer(it, userIcon) }
     }
-    editor.putUserData(CodeReviewCommentableEditorModel.KEY, model)
+
     try {
+      editor.putUserData(CodeReviewCommentableEditorModel.KEY, model)
+      editor.putUserData(CodeReviewNavigableEditorViewModel.KEY, model)
       awaitCancellation()
     }
     finally {
       editor.putUserData(CodeReviewCommentableEditorModel.KEY, null)
+      editor.putUserData(CodeReviewNavigableEditorViewModel.KEY, null)
     }
   }
 }