diff --git a/platform/collaboration-tools/api-dump-experimental.txt b/platform/collaboration-tools/api-dump-experimental.txt index 34f0ae600eb6..b058e62bc620 100644 --- a/platform/collaboration-tools/api-dump-experimental.txt +++ b/platform/collaboration-tools/api-dump-experimental.txt @@ -1036,6 +1036,7 @@ f:com.intellij.collaboration.ui.codereview.diff.viewer.DiffViewerUtilKt - a:getAdjustmentDisabledReason():kotlinx.coroutines.flow.StateFlow *e:com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel$Ranged$Adjustable$AdjustmentDisabledReason - java.lang.Enum +- sf:SINGLE_COMMIT_REVIEW:com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel$Ranged$Adjustable$AdjustmentDisabledReason - sf:SUGGESTED_CHANGE:com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel$Ranged$Adjustable$AdjustmentDisabledReason - s:getEntries():kotlin.enums.EnumEntries - s:valueOf(java.lang.String):com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel$Ranged$Adjustable$AdjustmentDisabledReason diff --git a/platform/collaboration-tools/resources/messages/CollaborationToolsBundle.properties b/platform/collaboration-tools/resources/messages/CollaborationToolsBundle.properties index 0abb46922cd5..e1c53e05028f 100644 --- a/platform/collaboration-tools/resources/messages/CollaborationToolsBundle.properties +++ b/platform/collaboration-tools/resources/messages/CollaborationToolsBundle.properties @@ -181,6 +181,7 @@ review.comments.discard.new.confirmation.title=Discard Comment review.comments.discard.new.confirmation=Are you sure you want to discard the unsaved comment? review.comments.code.outline.tooltip.explanation=Drag to select several lines for commenting review.comments.code.outline.tooltip.suggestion.disabling=The range of this comment is not adjustable as it contains suggestion +review.comments.code.outline.tooltip.commit.review.disabling=Comment range cannot be adjusted while reviewing a single commit review.comment.new.line.hint={0} to add new line review.comment.save=Save Changes diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlayRangeOutlineUtils.kt b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlayRangeOutlineUtils.kt index 0b08891475d0..dede62a9db45 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlayRangeOutlineUtils.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlayRangeOutlineUtils.kt @@ -4,6 +4,7 @@ import com.intellij.collaboration.async.collectScoped import com.intellij.collaboration.async.withInitial import com.intellij.collaboration.messages.CollaborationToolsBundle import com.intellij.collaboration.ui.codereview.comment.CommentedCodeFrameRenderer +import com.intellij.diff.util.DiffUtil import com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel.Ranged.Adjustable.AdjustmentDisabledReason import com.intellij.diff.util.LineRange import com.intellij.diff.util.Side @@ -187,6 +188,7 @@ private class ResizableOutlineHandler private constructor( handler.dragState.collectScoped { dragState -> if (dragState == null) { + if(initialRange.end > DiffUtil.getLineCount(editor.document)-1) return@collectScoped inlayRenderer.isVisible = true editor.showOutline(activeRangesTracker, initialRange) } @@ -328,7 +330,7 @@ private class ResizableOutlineHandler private constructor( * @return null if the line under [y] is not commentable or the new range is invalid (start after end) */ private fun DragState.withLineUnderYIfCommentable(y: Int): DragState? { - val lineUnderY = editor.xyToLogicalPosition(Point(0, y)).line + val lineUnderY = editor.xyToLogicalPosition(Point(0, y)).line.coerceIn(0, DiffUtil.getLineCount(editor.document)-1) val isCurrentBoundary = lineUnderY == line if (!canCreateComment(lineUnderY) && !isCurrentBoundary) return null @@ -364,6 +366,9 @@ private class ResizableOutlineHandler private constructor( AdjustmentDisabledReason.SUGGESTED_CHANGE -> { tooltipManager.showTooltip(component, point, OutlineTooltipManager.TooltipReason.SUGGESTION) } + AdjustmentDisabledReason.SINGLE_COMMIT_REVIEW -> { + tooltipManager.showTooltip(component, point, OutlineTooltipManager.TooltipReason.SINGLE_COMMIT_REVIEW) + } else -> { tooltipManager.showTooltip(component, point, OutlineTooltipManager.TooltipReason.MLC_EXPLANATION) setEditorCursor(resizeCursor) @@ -432,12 +437,14 @@ private class OutlineTooltipManager(private val editor: Editor) { enum class TooltipReason { SUGGESTION, - MLC_EXPLANATION; + MLC_EXPLANATION, + SINGLE_COMMIT_REVIEW; companion object { fun getTooltipMessage(tooltipReason: TooltipReason) = when (tooltipReason) { SUGGESTION -> CollaborationToolsBundle.message("review.comments.code.outline.tooltip.suggestion.disabling") MLC_EXPLANATION -> CollaborationToolsBundle.message("review.comments.code.outline.tooltip.explanation") + SINGLE_COMMIT_REVIEW -> CollaborationToolsBundle.message("review.comments.code.outline.tooltip.commit.review.disabling") } } } diff --git a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlaysModel.kt b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlaysModel.kt index 81cb20b1f2e6..4411b448e7a5 100644 --- a/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlaysModel.kt +++ b/platform/collaboration-tools/src/com/intellij/collaboration/ui/codereview/editor/CodeReviewEditorInlaysModel.kt @@ -26,7 +26,8 @@ interface CodeReviewInlayModel : EditorMappedViewModel { fun adjustRange(newStart: Int? = null, newEnd: Int? = null) enum class AdjustmentDisabledReason { - SUGGESTED_CHANGE + SUGGESTED_CHANGE, + SINGLE_COMMIT_REVIEW, } } } diff --git a/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/editorInlays.kt b/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/editorInlays.kt index 64a4d4bcea2b..d1ff19cb9652 100644 --- a/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/editorInlays.kt +++ b/plugins/github/github-core/src/org/jetbrains/plugins/github/pullrequest/ui/editor/editorInlays.kt @@ -23,8 +23,8 @@ internal sealed interface GHPREditorMappedComponentModel : CodeReviewInlayModel. } } - abstract class NewComment(val vm: VM) - : GHPREditorMappedComponentModel, CodeReviewInlayModel.Ranged.Adjustable + abstract class NewComment(val vm: VM) : GHPREditorMappedComponentModel, + CodeReviewInlayModel.Ranged.Adjustable abstract class AIComment(val vm: GHPRAICommentViewModel) : GHPREditorMappedComponentModel, Hideable { final override val key: Any = vm.key diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/api/dto/GitLabDiffPositionInput.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/api/dto/GitLabDiffPositionInput.kt index f0822106b625..aab48c739708 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/api/dto/GitLabDiffPositionInput.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/api/dto/GitLabDiffPositionInput.kt @@ -17,11 +17,16 @@ data class GitLabDiffPositionInput( GitLabDiffPositionInput( position.baseSha, position.startSha, - position.oldLineIndex?.inc(), + position.endLineIndexLeft?.inc(), position.headSha, - position.newLineIndex?.inc(), + position.endLineIndexRight?.inc(), position.paths, - position.lineRange + position.lineRange.let { range -> + LineRangeDTO( + start = range.start.toLinePositionDTO(position.paths), + end = range.end.toLinePositionDTO(position.paths) + ) + } ) } } @@ -36,6 +41,9 @@ data class LineRangeDTO( val end: LinePositionDTO, ) +/** + * Line position data with 1-based line indexes + */ data class LinePositionDTO( val lineCode: String?, val type: String?, diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabMergeRequestNewDiscussionPosition.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabMergeRequestNewDiscussionPosition.kt index 693634edf969..cf3eb1381499 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabMergeRequestNewDiscussionPosition.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabMergeRequestNewDiscussionPosition.kt @@ -5,57 +5,74 @@ import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.diff.util.Side import com.intellij.openapi.diff.impl.patch.TextFilePatch import com.intellij.openapi.diff.impl.patch.withoutContext +import com.intellij.util.io.DigestUtil import git4idea.changes.GitTextFilePatchWithHistory import org.jetbrains.plugins.gitlab.mergerequest.api.dto.DiffPathsInputDTO -import org.jetbrains.plugins.gitlab.mergerequest.api.dto.LineRangeDTO +import org.jetbrains.plugins.gitlab.mergerequest.api.dto.LinePositionDTO data class GitLabMergeRequestNewDiscussionPosition( val baseSha: String, val startSha: String, - val oldLineIndex: Int?, val headSha: String, - val newLineIndex: Int?, val paths: DiffPathsInputDTO, - val lineRange: LineRangeDTO? = null, + val lineRange: NewDiscussionLineRange, ) : GitLabNotePosition.WithLine { override val parentSha: String get() = baseSha override val sha: String get() = headSha override val filePathBefore: String? get() = paths.oldPath override val filePathAfter: String? get() = paths.newPath - override val lineIndexLeft: Int? get() = oldLineIndex - override val lineIndexRight: Int? get() = newLineIndex - override val startOldLine: Int? get() = lineRange?.start?.oldLine - override val startNewLine: Int? get() = lineRange?.start?.newLine - override val endOldLine: Int? get() = lineRange?.end?.oldLine - override val endNewLine: Int? get() = lineRange?.end?.newLine + override val lineIndexLeft: Int? get() = lineRange.end.oldLineIndex + override val lineIndexRight: Int? get() = lineRange.end.newLineIndex + override val startLineIndexLeft: Int? get() = lineRange.start.oldLineIndex + override val startLineIndexRight: Int? get() = lineRange.start.newLineIndex + override val endLineIndexLeft: Int? get() = lineRange.end.oldLineIndex + override val endLineIndexRight: Int? get() = lineRange.end.newLineIndex companion object { - fun calcFor(diffData: GitTextFilePatchWithHistory, location: DiffLineLocation): GitLabMergeRequestNewDiscussionPosition { + fun calcFor(diffData: GitTextFilePatchWithHistory, location: GitLabNoteLocation): GitLabMergeRequestNewDiscussionPosition { val patch = diffData.patch val startSha = patch.beforeVersionId!! val headSha = patch.afterVersionId!! val baseSha = if (diffData.isCumulative) diffData.fileHistory.findStartCommit()!! else startSha // Due to https://gitlab.com/gitlab-org/gitlab/-/issues/325161 we need line index for both sides for context lines - val otherSide = patch.transferToOtherSide(location) - val lineBefore = if (location.first == Side.LEFT) location.second else otherSide - val lineAfter = if (location.first == Side.RIGHT) location.second else otherSide + val (lineBefore, lineAfter) = beforeAndAfterLines(patch, location.side to location.lineIdx) + val (startLineBefore, startLineAfter) = beforeAndAfterLines(patch, location.startSide to location.startLineIdx) val pathBefore = patch.beforeName val pathAfter = patch.afterName + val lineRange = NewDiscussionLineRange( + start = NewDiscussionLinePosition( + side = location.startSide, + oldLineIndex = startLineBefore, + newLineIndex = startLineAfter, + ), + end = NewDiscussionLinePosition( + side = location.side, + oldLineIndex = lineBefore, + newLineIndex = lineAfter, + ) + ) + // Due to https://gitlab.com/gitlab-org/gitlab/-/issues/296829 we need base ref here val positionInput = GitLabMergeRequestNewDiscussionPosition( baseSha, startSha, - lineBefore, headSha, - lineAfter, - DiffPathsInputDTO(pathBefore, pathAfter) + DiffPathsInputDTO(pathBefore, pathAfter), + lineRange ) return positionInput } + private fun beforeAndAfterLines(patch: TextFilePatch, lineLocation: DiffLineLocation): Pair { + val otherSideStart = patch.transferToOtherSide(lineLocation) + val lineBefore = if (lineLocation.first == Side.LEFT) lineLocation.second else otherSideStart + val lineAfter = if (lineLocation.first == Side.RIGHT) lineLocation.second else otherSideStart + return lineBefore to lineAfter + } + private fun TextFilePatch.transferToOtherSide(location: DiffLineLocation): Int? { val (side, lineIndex) = location var lastEndBefore = 0 @@ -88,4 +105,38 @@ data class GitLabMergeRequestNewDiscussionPosition( } } } -} \ No newline at end of file +} + +/** + * Line position data with 0-based line indexes + */ +data class NewDiscussionLinePosition( + val side: Side, + val oldLineIndex: Int?, + val newLineIndex: Int?, +) { + fun toLinePositionDTO(paths: DiffPathsInputDTO): LinePositionDTO { + val oldLine = oldLineIndex?.inc() + val newLine = newLineIndex?.inc() + return LinePositionDTO( + lineCode = lineCode(side, paths.newPath, paths.oldPath, newLine, oldLine), + type = if (side == Side.RIGHT) "new" else "old", + oldLine = oldLine, + newLine = newLine + ) + } + + private fun lineCode(side: Side, pathAfter: String?, pathBefore: String?, lineAfter: Int?, lineBefore: Int?): String? { + val path = if (side == Side.RIGHT) pathAfter else pathBefore + if (path == null) return null + val hash = DigestUtil.sha1Hex(path) + val oldLine = lineBefore ?: lineAfter + val newLine = lineAfter ?: lineBefore + return "${hash}_${oldLine}_${newLine}" + } +} + +data class NewDiscussionLineRange( + val start: NewDiscussionLinePosition, + val end: NewDiscussionLinePosition, +) \ No newline at end of file diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabNotePosition.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabNotePosition.kt index 97de3a62b7fa..0eda61c01e0d 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabNotePosition.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/GitLabNotePosition.kt @@ -16,10 +16,10 @@ sealed interface GitLabNotePosition { interface WithLine : GitLabNotePosition { val lineIndexLeft: Int? val lineIndexRight: Int? - val startOldLine: Int? - val startNewLine: Int? - val endOldLine: Int? - val endNewLine: Int? + val startLineIndexLeft: Int? + val startLineIndexRight: Int? + val endLineIndexLeft: Int? + val endLineIndexRight: Int? } data class Text( @@ -29,10 +29,10 @@ sealed interface GitLabNotePosition { override val filePathAfter: String?, override val lineIndexLeft: Int?, override val lineIndexRight: Int?, - override val startOldLine: Int?, - override val startNewLine: Int?, - override val endOldLine: Int?, - override val endNewLine: Int?, + override val startLineIndexLeft: Int?, + override val startLineIndexRight: Int?, + override val endLineIndexLeft: Int?, + override val endLineIndexRight: Int?, ) : WithLine data class Image( diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/gitLabNotePositionUtil.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/gitLabNotePositionUtil.kt index 400fb6c2e1e1..e5612cf4cef8 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/gitLabNotePositionUtil.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/data/gitLabNotePositionUtil.kt @@ -7,8 +7,12 @@ import com.intellij.diff.util.Side import git4idea.changes.GitTextFilePatchWithHistory object GitLabNotePositionUtil { - fun getLocation(position: GitLabNotePosition.WithLine, contextSide: Side = Side.LEFT): DiffLineRange? = - when { + fun getLocation(position: GitLabNotePosition.WithLine, contextSide: Side = Side.LEFT): DiffLineRange? { + val forceRightSide = (position.lineIndexLeft == null && position.lineIndexRight != null) || + (position.startLineIndexLeft == null && position.startLineIndexRight != null) + + return when { + forceRightSide -> getRightSideLocation(position) position.lineIndexLeft != null && position.lineIndexRight != null -> when (contextSide) { Side.LEFT -> getLeftSideLocation(position) @@ -18,33 +22,37 @@ object GitLabNotePositionUtil { position.lineIndexRight != null -> getRightSideLocation(position) else -> null } - + } private fun getLeftSideLocation(position: GitLabNotePosition.WithLine): DiffLineRange? { - val startLine = position.startOldLine - val endLine = position.endOldLine + val (startSide, startLine) = position.startLineIndexLeft?.let { Side.LEFT to it } ?: (Side.RIGHT to position.startLineIndexRight) + val (endSide, endLine) = position.endLineIndexLeft?.let { Side.LEFT to it } ?: (Side.RIGHT to position.endLineIndexRight) - return if (startLine == null || endLine == null || endLine != position.lineIndexLeft) { // fallback to a single line + return if (startLine == null || endLine == null || // fallback to a single line + (endSide == Side.RIGHT && endLine != position.lineIndexRight) || + (endSide == Side.LEFT && endLine != position.lineIndexLeft)) { getSingleLineLocation(position.lineIndexLeft, position.lineIndexRight, Side.LEFT) ?.let { single -> DiffLineRange(single, single) } } else { - val startLoc = DiffLineLocation(Side.LEFT, startLine) - val endLoc = DiffLineLocation(Side.LEFT, endLine) + val startLoc = DiffLineLocation(startSide, startLine) + val endLoc = DiffLineLocation(endSide, endLine) DiffLineRange(startLoc, endLoc) } } private fun getRightSideLocation(position: GitLabNotePosition.WithLine): DiffLineRange? { - val startLine = position.startNewLine - val endLine = position.endNewLine + val (startSide, startLine) = position.startLineIndexRight?.let { Side.RIGHT to it } ?: (Side.LEFT to position.startLineIndexLeft) + val (endSide, endLine) = position.endLineIndexRight?.let { Side.RIGHT to it } ?: (Side.LEFT to position.endLineIndexLeft) - return if (startLine == null || endLine == null || endLine != position.lineIndexRight) { // fallback to a single line + return if (startLine == null || endLine == null || // fallback to a single line + (endSide == Side.RIGHT && endLine != position.lineIndexRight) || + (endSide == Side.LEFT && endLine != position.lineIndexLeft)) { getSingleLineLocation(position.lineIndexLeft, position.lineIndexRight, Side.RIGHT) ?.let { single -> DiffLineRange(single, single) } } else { - val startLoc = DiffLineLocation(Side.RIGHT, startLine) - val endLoc = DiffLineLocation(Side.RIGHT, endLine) + val startLoc = DiffLineLocation(startSide, startLine) + val endLoc = DiffLineLocation(endSide, endLine) DiffLineRange(startLoc, endLoc) } } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/diff/GitLabMergeRequestDiffExtension.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/diff/GitLabMergeRequestDiffExtension.kt index 8cbdb2a1eaf5..9ec7d28712b0 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/diff/GitLabMergeRequestDiffExtension.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/diff/GitLabMergeRequestDiffExtension.kt @@ -11,7 +11,14 @@ import com.intellij.collaboration.async.transformConsecutiveSuccesses import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.UnifiedCodeReviewItemPosition import com.intellij.collaboration.ui.codereview.diff.viewer.showCodeReview -import com.intellij.collaboration.ui.codereview.editor.* +import com.intellij.collaboration.ui.codereview.editor.CodeReviewActiveRangesTracker +import com.intellij.collaboration.ui.codereview.editor.CodeReviewCommentableEditorModel +import com.intellij.collaboration.ui.codereview.editor.CodeReviewComponentInlayRenderer +import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorGutterControlsModel +import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorInlayRangeOutlineUtils +import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorModel +import com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel.Ranged.Adjustable.AdjustmentDisabledReason +import com.intellij.collaboration.ui.codereview.editor.CodeReviewNavigableEditorViewModel import com.intellij.collaboration.ui.icon.IconsProvider import com.intellij.collaboration.util.ComputedResult import com.intellij.collaboration.util.Hideable @@ -31,10 +38,14 @@ import com.intellij.openapi.components.service import com.intellij.openapi.project.Project import com.intellij.util.cancelOnDispose import com.intellij.util.concurrency.annotations.RequiresEdt -import kotlinx.coroutines.* +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.withContext import org.jetbrains.plugins.gitlab.GitLabSettings import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO import org.jetbrains.plugins.gitlab.data.GitLabImageLoader @@ -161,28 +172,31 @@ private class DiffEditorModel( diffReviewVm.locationsWithDiscussions, diffReviewVm.locationsWithNewDiscussions ) { locationsWithDiscussions, locationsWithNewDiscussions -> - val linesWithDiscussions = locationsWithDiscussions.mapNotNullTo(mutableSetOf(), { locationToLine(it.side to it.lineIdx) }) - val linesWithNewDiscussions = locationsWithNewDiscussions.mapNotNullTo(mutableSetOf(), { locationToLine(it.side to it.lineIdx) }) - GutterState(linesWithDiscussions, linesWithNewDiscussions) + val linesWithDiscussions = locationsWithDiscussions.mapNotNullTo(mutableSetOf(), { locationToLine(it.first to it.second) }) + val linesWithNewDiscussions = locationsWithNewDiscussions.mapNotNullTo(mutableSetOf(), { locationToLine(it.first to it.second) }) + GutterState(linesWithDiscussions, linesWithNewDiscussions, lineToLocation) }.stateInNow(cs, null) override fun requestNewComment(lineIdx: Int) { - val loc = lineToLocation(lineIdx) ?: return + val loc = lineToLocation(lineIdx)?.let { + GitLabNoteLocation(it.first, it.second, it.first, it.second) + } ?: return diffReviewVm.requestNewDiscussion(loc, true) } override fun cancelNewComment(lineIdx: Int) { - val loc = lineToLocation(lineIdx) ?: return - diffReviewVm.cancelNewDiscussion(loc) + val loc = newDiscussions.value.find { it.line.value == lineIdx }?.vm?.location?.value ?: return + diffReviewVm.cancelNewDiscussion(loc.side to loc.lineIdx) } override fun requestNewComment(lineRange: LineRange) { - TODO("not implemented") + val loc1 = lineToLocation(lineRange.start) ?: return + val loc2 = lineToLocation(lineRange.end) ?: return + val loc = GitLabNoteLocation(loc1.first, loc1.second, loc2.first, loc2.second) + diffReviewVm.requestNewDiscussion(loc, true) } - override fun canCreateComment(lineRange: LineRange): Boolean { - TODO("not implemented") - } + override fun canCreateComment(lineRange: LineRange) = true override fun toggleComments(lineIdx: Int) { inlays.value.asSequence().filter { it.line.value == lineIdx }.filterIsInstance().syncOrToggleAll() @@ -248,17 +262,30 @@ private class DiffEditorModel( override val isVisible: StateFlow = MutableStateFlow(true) override val range: StateFlow = vm.location.mapState { it?.toLineRange(locationToLine) } override val line: StateFlow = range.mapState { it?.end } + override val adjustmentDisabledReason = MutableStateFlow( + AdjustmentDisabledReason.SINGLE_COMMIT_REVIEW.takeIf { !diffReviewVm.isCumulativeChange } + ) + override fun adjustRange(newStart: Int?, newEnd: Int?) { + if (newStart == null && newEnd == null) return + val range = range.value ?: return + val newRange = LineRange(newStart ?: range.start, newEnd ?: range.end) + val startLoc = lineToLocation(newRange.start) ?: return + val endLoc = lineToLocation(newRange.end) ?: return + vm.updateLineRange(startLoc, endLoc) + vm.requestFocus() + } override fun cancel() { - vm.location.value?.let(diffReviewVm::cancelNewDiscussion) + vm.location.value?.let { diffReviewVm.cancelNewDiscussion(it.side to it.lineIdx) } } } private data class GutterState( override val linesWithComments: Set, override val linesWithNewComments: Set, + val lineToLocation: (Int) -> DiffLineLocation? ) : CodeReviewEditorGutterControlsModel.ControlsState { - override fun isLineCommentable(lineIdx: Int): Boolean = true + override fun isLineCommentable(lineIdx: Int): Boolean = lineToLocation(lineIdx) != null } } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffDiscussionViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffDiscussionViewModel.kt index 1ca1735976a0..73d7e26448a9 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffDiscussionViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffDiscussionViewModel.kt @@ -5,18 +5,19 @@ import com.intellij.collaboration.async.combineState import com.intellij.collaboration.async.combineStates import com.intellij.collaboration.async.mapState import com.intellij.collaboration.ui.FocusableViewModel +import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.DiscussionsViewOption import com.intellij.diff.util.Side -import kotlinx.coroutines.flow.MutableStateFlow +import git4idea.changes.GitTextFilePatchWithHistory import kotlinx.coroutines.flow.StateFlow +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNewDiscussionPosition import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNoteLocation import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNotePosition import org.jetbrains.plugins.gitlab.mergerequest.data.mapToLocation import org.jetbrains.plugins.gitlab.mergerequest.ui.DiffDataMappedGitLabMergeRequestInlayModel -import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestDiscussionViewModel -import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestStandaloneDraftNoteViewModelBase -import org.jetbrains.plugins.gitlab.ui.comment.GitLabNoteViewModel -import org.jetbrains.plugins.gitlab.ui.comment.NewGitLabNoteViewModel +import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestDiscussionsViewModels +import org.jetbrains.plugins.gitlab.mergerequest.ui.review.mapToLocation +import org.jetbrains.plugins.gitlab.ui.comment.* interface DiffDataMappedGitLabMergeRequestDiffInlayViewModel : FocusableViewModel, @@ -57,12 +58,19 @@ class GitLabMergeRequestDiffDraftNoteViewModel internal constructor( } class GitLabMergeRequestDiffNewDiscussionViewModel internal constructor( - base: NewGitLabNoteViewModel, - originalLocation: GitLabNoteLocation, - discussionsViewOption: StateFlow, + private val base: NewGitLabNoteViewModelWithAdjustablePosition, + private val diffData: GitTextFilePatchWithHistory, + discussionsViewOption: StateFlow ) : NewGitLabNoteViewModel by base { - val location: StateFlow = MutableStateFlow(originalLocation) + val location: StateFlow = base.position.mapState { it.mapToLocation(diffData) } val isVisible: StateFlow = discussionsViewOption.mapState { it != DiscussionsViewOption.DONT_SHOW } + fun updateLineRange(startLocation: DiffLineLocation, endLocation: DiffLineLocation) { + val newLocation = GitLabNoteLocation(startLocation.first, startLocation.second, endLocation.first, endLocation.second) + val newPosition = GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition( + GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, newLocation), endLocation.first + ) + base.updatePosition(newPosition) + } } private fun mapPositionToDiffLine( diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffReviewViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffReviewViewModel.kt index 099ce5a18f2e..5da630d08a17 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffReviewViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/diff/GitLabMergeRequestDiffReviewViewModel.kt @@ -1,6 +1,7 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.plugins.gitlab.mergerequest.ui.diff +import com.intellij.collaboration.async.flatMapLatestEach import com.intellij.collaboration.async.stateInNow import com.intellij.collaboration.async.transformConsecutiveSuccesses import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation @@ -16,9 +17,9 @@ import com.intellij.openapi.project.Project import com.intellij.platform.util.coroutines.childScope import git4idea.changes.GitTextFilePatchWithHistory import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapNotNull import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO import org.jetbrains.plugins.gitlab.data.GitLabImageLoader import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequest @@ -40,8 +41,8 @@ interface GitLabMergeRequestDiffReviewViewModel { val draftDiscussions: StateFlow>> val newDiscussions: StateFlow> - val locationsWithDiscussions: StateFlow> - val locationsWithNewDiscussions: StateFlow> + val locationsWithDiscussions: StateFlow> + val locationsWithNewDiscussions: StateFlow> val avatarIconsProvider: IconsProvider val imageLoader: GitLabImageLoader @@ -53,10 +54,8 @@ interface GitLabMergeRequestDiffReviewViewModel { fun showDiffAtComment(commentId: String) - fun requestNewDiscussion(location: DiffLineLocation, focus: Boolean) - fun cancelNewDiscussion(location: DiffLineLocation) fun requestNewDiscussion(location: GitLabNoteLocation, focus: Boolean) - fun cancelNewDiscussion(location: GitLabNoteLocation) + fun cancelNewDiscussion(lineLocation: DiffLineLocation) fun markViewed() } @@ -87,48 +86,34 @@ internal class GitLabMergeRequestDiffReviewViewModelImpl( diffVm.draftDiscussions .transformConsecutiveSuccesses { filterInFile(change) } .stateInNow(cs, ComputedResult.loading()) - override val newDiscussions: StateFlow> = discussionsContainer.newDiscussions.map { - it.mapNotNull { (position, vm) -> - val location = position.mapToLocation(diffData) ?: return@mapNotNull null - GitLabMergeRequestDiffNewDiscussionViewModel(vm, location, discussionsViewOption) + override val newDiscussions: StateFlow> = + discussionsContainer.newDiscussions.map { newDiscussions -> + newDiscussions.map { vm -> + GitLabMergeRequestDiffNewDiscussionViewModel(vm, diffData, discussionsViewOption) } }.stateInNow(cs, emptyList()) - override val locationsWithDiscussions: StateFlow> = GitLabMergeRequestDiscussionUtil + override val locationsWithDiscussions: StateFlow> = GitLabMergeRequestDiscussionUtil .createDiscussionsPositionsFlow(mergeRequest, discussionsViewOption).toLocations { - it.mapToLocation(diffData, Side.LEFT) + it.mapToLocation(diffData, Side.LEFT)?.let { DiffLineLocation(it.side, it.lineIdx) } }.stateInNow(cs, emptySet()) - @OptIn(ExperimentalCoroutinesApi::class) - override val locationsWithNewDiscussions: StateFlow> = + override val locationsWithNewDiscussions: StateFlow> = discussionsContainer.newDiscussions - .map { - it.keys.mapNotNullTo(mutableSetOf()) { - it.mapToLocation(diffData) ?: return@mapNotNullTo null - } - } + .flatMapLatestEach { + it.position.mapNotNull { pos -> pos.mapToLocation(diffData)?.let { DiffLineLocation(it.side, it.lineIdx) } } + }.map { locations -> locations.toSet() } .stateInNow(cs, emptySet()) - override fun requestNewDiscussion(location: DiffLineLocation, focus: Boolean) { + override fun requestNewDiscussion(location: GitLabNoteLocation, focus: Boolean) { val position = GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, location).let { - GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition(it, location.first) + GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition(it, location.side) } discussionsContainer.requestNewDiscussion(position, focus) } - override fun cancelNewDiscussion(location: DiffLineLocation) { - val position = GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, location).let { - GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition(it, location.first) - } - discussionsContainer.cancelNewDiscussion(position) - } - - override fun requestNewDiscussion(location: GitLabNoteLocation, focus: Boolean) { - TODO("not implemented") - } - - override fun cancelNewDiscussion(location: GitLabNoteLocation) { - TODO("not implemented") + override fun cancelNewDiscussion(lineLocation: DiffLineLocation) { + discussionsContainer.cancelNewDiscussion(lineLocation) } override fun markViewed() { diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorDiscussionViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorDiscussionViewModel.kt index d1f2d9c061f9..83c7676f0357 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorDiscussionViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorDiscussionViewModel.kt @@ -5,20 +5,24 @@ import com.intellij.collaboration.async.combineState import com.intellij.collaboration.async.combineStates import com.intellij.collaboration.async.mapState import com.intellij.collaboration.ui.FocusableViewModel +import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.DiscussionsViewOption import com.intellij.collaboration.ui.codereview.editor.CodeReviewInlayModel import com.intellij.diff.util.Side -import kotlinx.coroutines.flow.MutableStateFlow +import git4idea.changes.GitTextFilePatchWithHistory import kotlinx.coroutines.flow.StateFlow import org.jetbrains.annotations.ApiStatus +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNewDiscussionPosition import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNoteLocation import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNotePosition import org.jetbrains.plugins.gitlab.mergerequest.data.mapToLocation import org.jetbrains.plugins.gitlab.mergerequest.ui.DiffDataMappedGitLabMergeRequestInlayModel +import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestDiscussionsViewModels +import org.jetbrains.plugins.gitlab.mergerequest.ui.review.mapToLocation import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestDiscussionViewModel import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestStandaloneDraftNoteViewModelBase import org.jetbrains.plugins.gitlab.ui.comment.GitLabNoteViewModel -import org.jetbrains.plugins.gitlab.ui.comment.NewGitLabNoteViewModel +import org.jetbrains.plugins.gitlab.ui.comment.NewGitLabNoteViewModelWithAdjustablePosition import java.util.* @ApiStatus.Internal @@ -68,14 +72,25 @@ class GitLabMergeRequestEditorDraftNoteViewModel internal constructor( @ApiStatus.Internal class GitLabMergeRequestEditorNewDiscussionViewModel internal constructor( - base: NewGitLabNoteViewModel, - originalLocation: GitLabNoteLocation, + private val base: NewGitLabNoteViewModelWithAdjustablePosition, + private val diffData: GitTextFilePatchWithHistory, discussionsViewOption: StateFlow, -) : NewGitLabNoteViewModel by base, CodeReviewInlayModel { +) : NewGitLabNoteViewModelWithAdjustablePosition by base, CodeReviewInlayModel { + val location: StateFlow = position.mapState { it.mapToLocation(diffData) } override val key: Any = "NEW_${UUID.randomUUID()}" - val location: StateFlow = MutableStateFlow(originalLocation) override val line: StateFlow = location.mapState { it?.lineIdx } override val isVisible: StateFlow = discussionsViewOption.mapState { it != DiscussionsViewOption.DONT_SHOW } + fun updateLineRange(startLocation: DiffLineLocation?, endLocation: DiffLineLocation?) { + val oldLocation = location.value ?: return + val newLocation = GitLabNoteLocation(startLocation?.first ?: oldLocation.startSide, + startLocation?.second ?: oldLocation.startLineIdx, + endLocation?.first ?: oldLocation.side, + endLocation?.second ?: oldLocation.lineIdx) + val newPosition = GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition( + GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, newLocation), Side.RIGHT + ) + updatePosition(newPosition) + } } private fun mapPositionToRightLocation( diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewFileViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewFileViewModel.kt index cfde60cadd6e..b8efe8b580c2 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewFileViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewFileViewModel.kt @@ -1,6 +1,7 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.plugins.gitlab.mergerequest.ui.editor +import com.intellij.collaboration.async.flatMapLatestEach import com.intellij.collaboration.async.mapState import com.intellij.collaboration.async.stateInNow import com.intellij.collaboration.async.transformConsecutiveSuccesses @@ -23,7 +24,6 @@ import git4idea.changes.GitTextFilePatchWithHistory import git4idea.changes.createVcsChange import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted @@ -32,11 +32,13 @@ import kotlinx.coroutines.flow.asSharedFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.stateIn import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO import org.jetbrains.plugins.gitlab.data.GitLabImageLoader import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequest import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNewDiscussionPosition +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNoteLocation import org.jetbrains.plugins.gitlab.mergerequest.data.mapToLocation import org.jetbrains.plugins.gitlab.mergerequest.ui.filterInFile import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestDiscussionsViewModels @@ -73,8 +75,8 @@ interface GitLabMergeRequestEditorReviewFileViewModel { fun getThreadPosition(noteTrackingId: String): Pair? fun requestThreadFocus(noteTrackingId: String) - fun requestNewDiscussion(line: Int, focus: Boolean) - fun cancelNewDiscussion(line: Int) + fun requestNewDiscussion(location: GitLabNoteLocation, focus: Boolean) + fun cancelNewDiscussion(lineLocation: DiffLineLocation) fun showDiff(line: Int?) } @@ -122,11 +124,13 @@ internal class GitLabMergeRequestEditorReviewFileViewModelImpl( .transformConsecutiveSuccesses { filterInFile(change) } .stateInNow(cs, ComputedResult.loading()) override val newDiscussions: StateFlow> = - discussionsContainer.newDiscussions.map { - it.mapNotNull { (position, vm) -> - val location = - position.mapToLocation(diffData)?.takeIf { it.startSide == Side.RIGHT && it.side == Side.RIGHT } ?: return@mapNotNull null - GitLabMergeRequestEditorNewDiscussionViewModel(vm, location, discussionsViewOption) + discussionsContainer.newDiscussions.flatMapLatestEach { vm -> + vm.position.map { pos -> vm to pos } + }.map { + it.mapNotNull { (vm, position) -> + val mappedLocation = position.mapToLocation(diffData) ?: return@mapNotNull null + if (mappedLocation.startSide != Side.RIGHT || mappedLocation.side != Side.RIGHT) return@mapNotNull null + GitLabMergeRequestEditorNewDiscussionViewModel(vm, diffData, discussionsViewOption) } }.stateInNow(cs, emptyList()) @@ -142,30 +146,22 @@ internal class GitLabMergeRequestEditorReviewFileViewModelImpl( override val canComment: StateFlow = discussionsViewOption.mapState { it != DiscussionsViewOption.DONT_SHOW } - @OptIn(ExperimentalCoroutinesApi::class) override val linesWithNewDiscussions: StateFlow> = - discussionsContainer.newDiscussions - .map { - it.keys.mapNotNullTo(mutableSetOf()) { - it.mapToLocation(diffData)?.takeIf { - it.startSide == Side.RIGHT && it.side == Side.RIGHT - }?.lineIdx ?: return@mapNotNullTo null - } + discussionsContainer.newDiscussions.flatMapLatestEach { + it.position.mapNotNull { pos -> + pos.mapToLocation(diffData)?.takeIf { loc -> loc.startSide == Side.RIGHT && loc.side == Side.RIGHT }?.lineIdx } - .stateInNow(cs, emptySet()) + }.map { lines -> lines.toSet() }.stateInNow(cs, emptySet()) - override fun requestNewDiscussion(line: Int, focus: Boolean) { - val position = GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, DiffLineLocation(Side.RIGHT, line)).let { + override fun requestNewDiscussion(location: GitLabNoteLocation, focus: Boolean) { + val position = GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, location).let { GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition(it, Side.RIGHT) } discussionsContainer.requestNewDiscussion(position, focus) } - override fun cancelNewDiscussion(line: Int) { - val position = GitLabMergeRequestNewDiscussionPosition.calcFor(diffData, DiffLineLocation(Side.RIGHT, line)).let { - GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition(it, Side.RIGHT) - } - discussionsContainer.cancelNewDiscussion(position) + override fun cancelNewDiscussion(lineLocation: DiffLineLocation) { + discussionsContainer.cancelNewDiscussion(lineLocation) } override fun lookupNextComment(line: Int, additionalIsVisible: (String) -> Boolean): String? = diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewUIModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewUIModel.kt index d083ee2229f3..0ed31044dd99 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewUIModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/GitLabMergeRequestEditorReviewUIModel.kt @@ -13,6 +13,7 @@ import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorGutterCha import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorGutterControlsModel import com.intellij.collaboration.ui.codereview.editor.CodeReviewEditorInlaysModel import com.intellij.collaboration.ui.codereview.editor.CodeReviewNavigableEditorViewModel +import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.editor.MutableCodeReviewEditorGutterChangesModel import com.intellij.collaboration.ui.codereview.editor.ReviewInEditorUtil import com.intellij.collaboration.ui.codereview.editor.asLst @@ -24,6 +25,7 @@ import com.intellij.collaboration.util.getOrNull import com.intellij.collaboration.util.syncOrToggleAll import com.intellij.diff.util.LineRange import com.intellij.diff.util.Range +import com.intellij.diff.util.Side import com.intellij.openapi.Disposable import com.intellij.openapi.project.Project import com.intellij.util.cancelOnDispose @@ -100,30 +102,33 @@ internal class GitLabMergeRequestEditorReviewUIModel internal constructor( override fun requestNewComment(lineIdx: Int) { val ranges = postReviewRanges.value ?: return val originalLine = ReviewInEditorUtil.transferLineFromAfter(ranges, lineIdx)?.takeIf { it >= 0 } ?: return - fileVm.requestNewDiscussion(originalLine, true) + val location = GitLabNoteLocation(Side.RIGHT, originalLine, Side.RIGHT, originalLine) + fileVm.requestNewDiscussion(location, true) } override fun cancelNewComment(lineIdx: Int) { val ranges = postReviewRanges.value ?: return val originalLine = ReviewInEditorUtil.transferLineFromAfter(ranges, lineIdx)?.takeIf { it >= 0 } ?: return - fileVm.cancelNewDiscussion(originalLine) + fileVm.cancelNewDiscussion(Side.RIGHT to originalLine) } override fun requestNewComment(lineRange: LineRange) { - TODO("not implemented") + val ranges = postReviewRanges.value ?: return + val startLine = ReviewInEditorUtil.transferLineFromAfter(ranges, lineRange.start)?.takeIf { it >= 0 } ?: return + val originalLine = ReviewInEditorUtil.transferLineFromAfter(ranges, lineRange.end)?.takeIf { it >= 0 } ?: return + val location = GitLabNoteLocation(Side.RIGHT, startLine, Side.RIGHT, originalLine) + fileVm.requestNewDiscussion(location, true) } - override fun canCreateComment(lineRange: LineRange): Boolean { - TODO("not implemented") - } + override fun canCreateComment(lineRange: LineRange) = true override fun toggleComments(lineIdx: Int) { inlays.value.asSequence().filter { it.line.value == lineIdx }.filterIsInstance().syncOrToggleAll() GitLabStatistics.logToggledComments(project) } - fun cancelNewDiscussion(originalLine: Int) { - fileVm.cancelNewDiscussion(originalLine) + fun cancelNewDiscussion(lineLocation: DiffLineLocation) { + fileVm.cancelNewDiscussion(lineLocation) } override fun getBaseContent(lines: LineRange): String? = fileVm.getBaseContent(lines) @@ -228,14 +233,30 @@ internal class GitLabMergeRequestEditorReviewUIModel internal constructor( override val line: StateFlow = range.mapState { it?.end } } - private inner class ShiftedNewDiscussion(vm: GitLabMergeRequestEditorNewDiscussionViewModel) - : GitLabMergeRequestEditorMappedComponentModel.NewDiscussion(vm) { + private inner class ShiftedNewDiscussion(vm: GitLabMergeRequestEditorNewDiscussionViewModel) : + GitLabMergeRequestEditorMappedComponentModel.NewDiscussion(vm) { override val key: Any = vm.key override val isVisible: StateFlow = MutableStateFlow(true) override val range: StateFlow = vm.location.shiftLineRange() override val line: StateFlow = range.mapState { it?.end } + override val adjustmentDisabledReason = MutableStateFlow(null) + override fun adjustRange(newStart: Int?, newEnd: Int?) { + if (newStart == null && newEnd == null) return + val ranges = postReviewRanges.value ?: emptyList() + val transferredStart = newStart?.let { + val startLine = ReviewInEditorUtil.transferLineFromAfter(ranges, it)?.takeIf { it >= 0 } ?: return@let null + Side.RIGHT to startLine + } + val transferredEnd = newEnd?.let { + val endLine = ReviewInEditorUtil.transferLineFromAfter(ranges, it)?.takeIf { it >= 0 } ?: return@let null + Side.RIGHT to endLine + } + vm.updateLineRange(transferredStart, transferredEnd) + vm.requestFocus() + } + override fun cancel() { - vm.line.value?.let(::cancelNewDiscussion) + vm.location.value?.let { cancelNewDiscussion(it.side to it.lineIdx) } } } } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/editorInlays.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/editorInlays.kt index cc81c6523589..9d12f507751a 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/editorInlays.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/editor/editorInlays.kt @@ -30,8 +30,8 @@ internal sealed interface GitLabMergeRequestEditorMappedComponentModel : CodeRev } } - abstract class NewDiscussion(override val vm: VM) - : GitLabMergeRequestEditorMappedComponentModel { + abstract class NewDiscussion(override val vm: VM) : GitLabMergeRequestEditorMappedComponentModel, + CodeReviewInlayModel.Ranged.Adjustable { abstract fun cancel() } } \ No newline at end of file diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/review/GitLabMergeRequestDiscussionsViewModels.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/review/GitLabMergeRequestDiscussionsViewModels.kt index 14c33418fa5a..356d7e5bedee 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/review/GitLabMergeRequestDiscussionsViewModels.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/ui/review/GitLabMergeRequestDiscussionsViewModels.kt @@ -1,7 +1,12 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package org.jetbrains.plugins.gitlab.mergerequest.ui.review -import com.intellij.collaboration.async.* +import com.intellij.collaboration.async.cancelAndJoinSilently +import com.intellij.collaboration.async.mapFiltered +import com.intellij.collaboration.async.mapState +import com.intellij.collaboration.async.mapStatefulToStateful +import com.intellij.collaboration.async.stateInNow +import com.intellij.collaboration.async.transformConsecutiveSuccesses import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.UnifiedCodeReviewItemPosition import com.intellij.collaboration.util.ComputedResult @@ -17,19 +22,41 @@ import git4idea.changes.findCumulativeChange import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.* +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.update +import kotlinx.coroutines.flow.updateAndGet import kotlinx.coroutines.launch import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO -import org.jetbrains.plugins.gitlab.mergerequest.data.* +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequest +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNewDiscussionPosition +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNoteLocation +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabNotePosition +import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabProject +import org.jetbrains.plugins.gitlab.mergerequest.data.filePath +import org.jetbrains.plugins.gitlab.mergerequest.data.mapToLeftSideLine +import org.jetbrains.plugins.gitlab.mergerequest.data.mapToLocation +import org.jetbrains.plugins.gitlab.mergerequest.data.mapToRightSideLine import org.jetbrains.plugins.gitlab.ui.GitLabMarkdownToHtmlConverter -import org.jetbrains.plugins.gitlab.ui.comment.* +import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestDiscussionViewModel +import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestDiscussionViewModelBase +import org.jetbrains.plugins.gitlab.ui.comment.GitLabMergeRequestStandaloneDraftNoteViewModelBase +import org.jetbrains.plugins.gitlab.ui.comment.GitLabNoteEditingViewModel +import org.jetbrains.plugins.gitlab.ui.comment.NewGitLabNoteViewModelWithAdjustablePosition +import org.jetbrains.plugins.gitlab.ui.comment.onDoneIn import java.time.Instant.EPOCH -import java.util.Date -import java.util.TreeSet +import java.util.* private typealias DiscussionsFlow = StateFlow>> private typealias DraftNotesFlow = StateFlow>> -private typealias NewDiscussionsFlow = Flow> +private typealias NewDiscussionsFlow = StateFlow> /** * Represents the discussions and notes in a merge request at a conceptual level. @@ -51,7 +78,7 @@ interface GitLabMergeRequestDiscussionsViewModels { fun lookupPreviousComment(currentThreadId: String, isVisible: (String) -> Boolean): String? fun requestNewDiscussion(position: NewDiscussionPosition, focus: Boolean) - fun cancelNewDiscussion(position: NewDiscussionPosition) + fun cancelNewDiscussion(lineLocation: DiffLineLocation) class NewDiscussionPosition(val position: GitLabMergeRequestNewDiscussionPosition, val side: Side) { override fun equals(other: Any?): Boolean { @@ -99,8 +126,7 @@ internal class GitLabMergeRequestDiscussionsViewModelsImpl( } .stateInNow(cs, ComputedResult.loading()) - private val _newDiscussions = - MutableStateFlow>(emptyMap()) + private val _newDiscussions = MutableStateFlow>(emptyList()) override val newDiscussions: NewDiscussionsFlow = _newDiscussions.asStateFlow() @OptIn(ExperimentalCoroutinesApi::class) @@ -118,9 +144,8 @@ internal class GitLabMergeRequestDiscussionsViewModelsImpl( } val newDiscussionsData = _newDiscussions - .mapState { it.entries } - .mapStatefulToStateful { (position, note) -> - IntermediateDiscussionData(note.trackingId, Date(), 0, MutableStateFlow(position.position)) + .mapStatefulToStateful { note -> + IntermediateDiscussionData(note.trackingId, Date(), 0, MutableStateFlow(note.position.value.position)) } val allDiscussions = @@ -149,32 +174,49 @@ internal class GitLabMergeRequestDiscussionsViewModelsImpl( override fun requestNewDiscussion(position: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition, focus: Boolean) { _newDiscussions.updateAndGet { currentNewDiscussions -> - if (!currentNewDiscussions.containsKey(position) && mergeRequest.canAddNotes) { - val vm = GitLabNoteEditingViewModel.forNewDiffNote(cs, project, projectData, mergeRequest, currentUser, position.position).apply { + if (!currentNewDiscussions.any { it.position.value == position.position } && mergeRequest.canAddNotes) { + val vm = GitLabNoteEditingViewModel.forNewDiffNote(cs, project, projectData, mergeRequest, currentUser, position).apply { onDoneIn(cs) { - cancelNewDiscussion(position) + cancelNewDiscussion(this) } } - currentNewDiscussions + (position to vm) + currentNewDiscussions + (vm) } else { currentNewDiscussions } }.apply { if (focus) { - get(position)?.requestFocus() + find { it.position.value == position.position }?.requestFocus() } } } - override fun cancelNewDiscussion(position: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition) { + private fun cancelNewDiscussion(oldVm: NewGitLabNoteViewModelWithAdjustablePosition) { _newDiscussions.update { - val oldVm = it[position] - val newMap = it - position + val newList = it - oldVm cs.launch { - oldVm?.destroy() + oldVm.destroy() } - newMap + newList + } + } + + override fun cancelNewDiscussion(lineLocation: DiffLineLocation) { + _newDiscussions.update { + val oldVm = it.find { vm -> + val position = vm.position.value + val loc = position.side to if (position.side == Side.LEFT) + position.position.endLineIndexLeft + else + position.position.endLineIndexRight + loc == lineLocation + } ?: return@update it + val newList = it - oldVm + cs.launch { + oldVm.destroy() + } + newList } } diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestDiscussionUtil.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestDiscussionUtil.kt index 640e66b9b702..6f95cd58db5a 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestDiscussionUtil.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/mergerequest/util/GitLabMergeRequestDiscussionUtil.kt @@ -2,6 +2,7 @@ package org.jetbrains.plugins.gitlab.mergerequest.util import com.intellij.collaboration.async.flatMapLatestEach +import com.intellij.collaboration.ui.codereview.diff.DiffLineLocation import com.intellij.collaboration.ui.codereview.diff.DiscussionsViewOption import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine @@ -67,5 +68,5 @@ internal object GitLabMergeRequestDiscussionUtil { fun Flow>.toLines(mapper: (GitLabNotePosition) -> Int?): Flow> = map { it.mapNotNullTo(mutableSetOf(), mapper) } -fun Flow>.toLocations(mapper: (GitLabNotePosition) -> GitLabNoteLocation?): Flow> = +fun Flow>.toLocations(mapper: (GitLabNotePosition) -> DiffLineLocation?): Flow> = map { it.mapNotNullTo(mutableSetOf(), mapper) } \ No newline at end of file diff --git a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/ui/comment/GitLabNoteEditingViewModel.kt b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/ui/comment/GitLabNoteEditingViewModel.kt index 288ad994c2f8..6528a32aeeb0 100644 --- a/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/ui/comment/GitLabNoteEditingViewModel.kt +++ b/plugins/gitlab/gitlab-core/src/org/jetbrains/plugins/gitlab/ui/comment/GitLabNoteEditingViewModel.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.launch @@ -25,15 +26,15 @@ import org.jetbrains.plugins.gitlab.api.dto.GitLabUserDTO import org.jetbrains.plugins.gitlab.mergerequest.GitLabMergeRequestsPreferences import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabDiscussion import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequest -import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabMergeRequestNewDiscussionPosition import org.jetbrains.plugins.gitlab.mergerequest.data.GitLabProject import org.jetbrains.plugins.gitlab.mergerequest.data.MutableGitLabNote +import org.jetbrains.plugins.gitlab.mergerequest.ui.review.GitLabMergeRequestDiscussionsViewModels import org.jetbrains.plugins.gitlab.ui.comment.GitLabCodeReviewSubmittableTextViewModel.FileUploadResult import org.jetbrains.plugins.gitlab.upload.GitLabUploadFileUtil import org.jetbrains.plugins.gitlab.util.GitLabStatistics import java.awt.Image import java.nio.file.Path -import java.util.UUID +import java.util.* import javax.swing.Action interface GitLabCodeReviewSubmittableTextViewModel : CodeReviewSubmittableTextViewModel { @@ -73,8 +74,8 @@ interface GitLabNoteEditingViewModel : GitLabCodeReviewSubmittableTextViewModel projectData: GitLabProject, mergeRequest: GitLabMergeRequest, currentUser: GitLabUserDTO, - position: GitLabMergeRequestNewDiscussionPosition - ): NewGitLabNoteViewModel = + position: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition, + ): NewGitLabNoteViewModelWithAdjustablePosition = NewDiffGitLabNoteViewModel(project, parentCs, "", projectData, mergeRequest, currentUser, position) internal fun forReplyNote( @@ -151,6 +152,11 @@ interface NewGitLabNoteViewModel : fun submitAsDraft() } +interface NewGitLabNoteViewModelWithAdjustablePosition : NewGitLabNoteViewModel { + val position: StateFlow + fun updatePosition(newPosition: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition) +} + private abstract class NewGitLabNoteViewModelBase( project: Project, parentCs: CoroutineScope, @@ -198,17 +204,26 @@ private class NewStandaloneGitLabNoteViewModel(project: Project, override suspend fun doSubmitAsDraft(text: String) = mergeRequest.addDraftNote(text) } -private class NewDiffGitLabNoteViewModel(project: Project, - parentCs: CoroutineScope, - initialText: String, - projectData: GitLabProject, - private val mergeRequest: GitLabMergeRequest, - currentUser: GitLabUserDTO, - private val position: GitLabMergeRequestNewDiscussionPosition) - : NewGitLabNoteViewModelBase(project, parentCs, projectData, initialText, currentUser, ) { +private class NewDiffGitLabNoteViewModel( + project: Project, + parentCs: CoroutineScope, + initialText: String, + projectData: GitLabProject, + private val mergeRequest: GitLabMergeRequest, + currentUser: GitLabUserDTO, + position: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition, +) : NewGitLabNoteViewModelBase(project, parentCs, projectData, initialText, currentUser), NewGitLabNoteViewModelWithAdjustablePosition { + private val _position = MutableStateFlow(position) + override val position = _position.asStateFlow() override val canSubmitAsDraft: Boolean = mergeRequest.canAddPositionalDraftNotes - override suspend fun doSubmit(text: String) = mergeRequest.addNote(position, text) - override suspend fun doSubmitAsDraft(text: String) = mergeRequest.addDraftNote(position, text) + override fun updatePosition(newPosition: GitLabMergeRequestDiscussionsViewModels.NewDiscussionPosition) { + _position.value = newPosition + } + + override suspend fun doSubmit(text: String) = + mergeRequest.addNote(position.value.position, text) + + override suspend fun doSubmitAsDraft(text: String) = mergeRequest.addDraftNote(position.value.position, text) } private class NewReplyGitLabNoteViewModel(project: Project,