mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-01-04 17:20:55 +07:00
[github] Add way to forcefully guestimate the LEFT or RIGHT line number of a comment
Includes declarativish testing of line mapping. GitOrigin-RevId: fc792139f4d40a2f5c88f09342030837be9839c8
This commit is contained in:
committed by
intellij-monorepo-bot
parent
b93d13d44c
commit
ab8b1813a2
@@ -29,6 +29,7 @@ jvm_library(
|
|||||||
"//platform/platform-impl:ide-impl_test_lib",
|
"//platform/platform-impl:ide-impl_test_lib",
|
||||||
"//platform/util/diff",
|
"//platform/util/diff",
|
||||||
"//platform/vcs-api/vcs-api-core:vcs-core",
|
"//platform/vcs-api/vcs-api-core:vcs-core",
|
||||||
|
"//platform/vcs-api/vcs-api-core:vcs-core_test_lib",
|
||||||
"@lib//:mockito",
|
"@lib//:mockito",
|
||||||
"//platform/analysis-api:analysis",
|
"//platform/analysis-api:analysis",
|
||||||
"//platform/editor-ui-ex:editor-ex",
|
"//platform/editor-ui-ex:editor-ex",
|
||||||
|
|||||||
@@ -67,6 +67,7 @@ jvm_library(
|
|||||||
"//platform/util/jdom",
|
"//platform/util/jdom",
|
||||||
"//platform/analysis-impl",
|
"//platform/analysis-impl",
|
||||||
"//platform/vcs-api/vcs-api-core:vcs-core",
|
"//platform/vcs-api/vcs-api-core:vcs-core",
|
||||||
|
"//platform/vcs-api/vcs-api-core:vcs-core_test_lib",
|
||||||
"//platform/platform-impl:ide-impl",
|
"//platform/platform-impl:ide-impl",
|
||||||
"//platform/platform-impl:ide-impl_test_lib",
|
"//platform/platform-impl:ide-impl_test_lib",
|
||||||
"//platform/diff-api:diff",
|
"//platform/diff-api:diff",
|
||||||
|
|||||||
@@ -1,5 +1,5 @@
|
|||||||
### auto-generated section `build intellij.platform.vcs.core` start
|
### auto-generated section `build intellij.platform.vcs.core` start
|
||||||
load("@rules_jvm//:jvm.bzl", "jvm_library", "jvm_resources")
|
load("@rules_jvm//:jvm.bzl", "jvm_library", "jvm_resources", "jvm_test")
|
||||||
|
|
||||||
jvm_resources(
|
jvm_resources(
|
||||||
name = "vcs-core_resources",
|
name = "vcs-core_resources",
|
||||||
@@ -23,4 +23,29 @@ jvm_library(
|
|||||||
],
|
],
|
||||||
runtime_deps = [":vcs-core_resources"]
|
runtime_deps = [":vcs-core_resources"]
|
||||||
)
|
)
|
||||||
|
|
||||||
|
jvm_library(
|
||||||
|
name = "vcs-core_test_lib",
|
||||||
|
visibility = ["//visibility:public"],
|
||||||
|
srcs = glob(["test/**/*.kt", "test/**/*.java"], allow_empty = True),
|
||||||
|
associates = [":vcs-core"],
|
||||||
|
deps = [
|
||||||
|
"//platform/core-api:core",
|
||||||
|
"//platform/util",
|
||||||
|
"//platform/editor-ui-api:editor-ui",
|
||||||
|
"//platform/ide-core",
|
||||||
|
"//platform/diff-api:diff",
|
||||||
|
"//platform/util/diff",
|
||||||
|
"@lib//:kotlin-stdlib",
|
||||||
|
"//libraries/junit5",
|
||||||
|
"//libraries/junit5-params",
|
||||||
|
"//libraries/assertj-core",
|
||||||
|
],
|
||||||
|
runtime_deps = [":vcs-core_resources"]
|
||||||
|
)
|
||||||
|
|
||||||
|
jvm_test(
|
||||||
|
name = "vcs-core_test",
|
||||||
|
runtime_deps = [":vcs-core_test_lib"]
|
||||||
|
)
|
||||||
### auto-generated section `build intellij.platform.vcs.core` end
|
### auto-generated section `build intellij.platform.vcs.core` end
|
||||||
@@ -5,6 +5,7 @@
|
|||||||
<content url="file://$MODULE_DIR$">
|
<content url="file://$MODULE_DIR$">
|
||||||
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
|
<sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
|
||||||
<sourceFolder url="file://$MODULE_DIR$/resources" type="java-resource" />
|
<sourceFolder url="file://$MODULE_DIR$/resources" type="java-resource" />
|
||||||
|
<sourceFolder url="file://$MODULE_DIR$/test" isTestSource="true" />
|
||||||
</content>
|
</content>
|
||||||
<orderEntry type="inheritedJdk" />
|
<orderEntry type="inheritedJdk" />
|
||||||
<orderEntry type="sourceFolder" forTests="false" />
|
<orderEntry type="sourceFolder" forTests="false" />
|
||||||
@@ -15,5 +16,8 @@
|
|||||||
<orderEntry type="module" module-name="intellij.platform.diff" />
|
<orderEntry type="module" module-name="intellij.platform.diff" />
|
||||||
<orderEntry type="module" module-name="intellij.platform.util.diff" />
|
<orderEntry type="module" module-name="intellij.platform.util.diff" />
|
||||||
<orderEntry type="library" name="kotlin-stdlib" level="project" />
|
<orderEntry type="library" name="kotlin-stdlib" level="project" />
|
||||||
|
<orderEntry type="module" module-name="intellij.libraries.junit5" scope="TEST" />
|
||||||
|
<orderEntry type="module" module-name="intellij.libraries.junit5.params" scope="TEST" />
|
||||||
|
<orderEntry type="module" module-name="intellij.libraries.assertj.core" scope="TEST" />
|
||||||
</component>
|
</component>
|
||||||
</module>
|
</module>
|
||||||
@@ -0,0 +1,37 @@
|
|||||||
|
// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||||
|
package com.intellij.openapi.diff.impl.patch
|
||||||
|
|
||||||
|
import com.intellij.diff.util.Range
|
||||||
|
import com.intellij.openapi.diff.impl.patch.PatchHunkUtil.getChangeOnlyRanges
|
||||||
|
import org.assertj.core.api.Assertions.assertThat
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource
|
||||||
|
|
||||||
|
class PatchHunkUtilTest {
|
||||||
|
@ParameterizedTest(name = "{0}")
|
||||||
|
@MethodSource("simpleRanges")
|
||||||
|
fun `getChangeOnlyRanges for a simple one-range hunk calculates that one range`(range: Range) {
|
||||||
|
val hunk = PatchHunk(range.start1, range.end1, range.start2, range.end2).apply {
|
||||||
|
repeat(range.end1 - range.start1) { addLine(REMOVE_LINE) }
|
||||||
|
repeat(range.end2 - range.start2) { addLine(ADD_LINE) }
|
||||||
|
}
|
||||||
|
|
||||||
|
assertThat(getChangeOnlyRanges(hunk))
|
||||||
|
.hasSize(1)
|
||||||
|
.first().isEqualTo(range)
|
||||||
|
}
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
@JvmStatic
|
||||||
|
fun simpleRanges(): Array<Range> = arrayOf(
|
||||||
|
Range(0, 1, 0, 1),
|
||||||
|
Range(0, 0, 0, 10),
|
||||||
|
Range(0, 5, 0, 10),
|
||||||
|
Range(3, 5, 3, 3),
|
||||||
|
Range(5, 5, 3, 4),
|
||||||
|
)
|
||||||
|
|
||||||
|
val REMOVE_LINE = PatchLine(PatchLine.Type.REMOVE, "").apply { isSuppressNewLine = true }
|
||||||
|
val ADD_LINE = PatchLine(PatchLine.Type.ADD, "").apply { isSuppressNewLine = true }
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -52,6 +52,7 @@ jvm_library(
|
|||||||
"//platform/lang-impl",
|
"//platform/lang-impl",
|
||||||
"//platform/vcs-impl/lang",
|
"//platform/vcs-impl/lang",
|
||||||
"//platform/vcs-api/vcs-api-core:vcs-core",
|
"//platform/vcs-api/vcs-api-core:vcs-core",
|
||||||
|
"//platform/vcs-api/vcs-api-core:vcs-core_test_lib",
|
||||||
"//platform/vcs-api:vcs",
|
"//platform/vcs-api:vcs",
|
||||||
"//platform/core-api:core",
|
"//platform/core-api:core",
|
||||||
"//platform/ide-core-impl",
|
"//platform/ide-core-impl",
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ jvm_library(
|
|||||||
"//platform/util",
|
"//platform/util",
|
||||||
"//platform/ide-core",
|
"//platform/ide-core",
|
||||||
"//platform/vcs-api/vcs-api-core:vcs-core",
|
"//platform/vcs-api/vcs-api-core:vcs-core",
|
||||||
|
"//platform/vcs-api/vcs-api-core:vcs-core_test_lib",
|
||||||
"//platform/vcs-log/api:vcs-log",
|
"//platform/vcs-log/api:vcs-log",
|
||||||
"//platform/dvcs-impl:vcs-dvcs-impl",
|
"//platform/dvcs-impl:vcs-dvcs-impl",
|
||||||
"//platform/dvcs-impl:vcs-dvcs-impl_test_lib",
|
"//platform/dvcs-impl:vcs-dvcs-impl_test_lib",
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ jvm_library(
|
|||||||
"//plugins/git4idea:vcs-git_test_lib",
|
"//plugins/git4idea:vcs-git_test_lib",
|
||||||
"//plugins/git4idea/shared",
|
"//plugins/git4idea/shared",
|
||||||
"//platform/vcs-api/vcs-api-core:vcs-core",
|
"//platform/vcs-api/vcs-api-core:vcs-core",
|
||||||
|
"//platform/vcs-api/vcs-api-core:vcs-core_test_lib",
|
||||||
"//platform/vcs-log/api:vcs-log",
|
"//platform/vcs-log/api:vcs-log",
|
||||||
"//platform/vcs-log/impl",
|
"//platform/vcs-log/impl",
|
||||||
"//platform/vcs-log/impl:impl_test_lib",
|
"//platform/vcs-log/impl:impl_test_lib",
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ interface GitFileHistory : Comparator<String> {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Retrieve a chain of patches between commits [parent] and [child]
|
* Retrieve a chain of patches between commits [parent] and [child]
|
||||||
|
* Including [child], but excluding [parent]
|
||||||
*/
|
*/
|
||||||
fun getPatchesBetween(parent: String, child: String): List<TextFilePatch>
|
fun getPatchesBetween(parent: String, child: String): List<TextFilePatch>
|
||||||
}
|
}
|
||||||
@@ -8,6 +8,7 @@ import com.intellij.openapi.diagnostic.logger
|
|||||||
import com.intellij.openapi.diff.impl.patch.PatchHunkUtil
|
import com.intellij.openapi.diff.impl.patch.PatchHunkUtil
|
||||||
import com.intellij.openapi.diff.impl.patch.TextFilePatch
|
import com.intellij.openapi.diff.impl.patch.TextFilePatch
|
||||||
import org.jetbrains.annotations.ApiStatus
|
import org.jetbrains.annotations.ApiStatus
|
||||||
|
import kotlin.math.floor
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Holds the [patch] between two commits in [fileHistory]
|
* Holds the [patch] between two commits in [fileHistory]
|
||||||
@@ -24,6 +25,32 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo
|
|||||||
*/
|
*/
|
||||||
fun contains(commitSha: String, filePath: String): Boolean = fileHistory.contains(commitSha, filePath)
|
fun contains(commitSha: String, filePath: String): Boolean = fileHistory.contains(commitSha, filePath)
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @return `null` only if the commit cannot be found.
|
||||||
|
* Otherwise, this function returns the most accurate line number on the given side of the patch history we could derive.
|
||||||
|
* If the line is removed during any patch, its location information can no longer be retrieved exactly, so instead, we
|
||||||
|
* continue with the topmost line of the changed hunk.
|
||||||
|
*/
|
||||||
|
fun forcefullyMapLine(fromCommitSha: String, lineIndex: Int, side: Side): Int? {
|
||||||
|
// map to merge base, not left revision
|
||||||
|
val beforeSha = if (isCumulative) fileHistory.findStartCommit()!! else patch.beforeVersionId!!
|
||||||
|
val afterSha = patch.afterVersionId!!
|
||||||
|
|
||||||
|
if (fromCommitSha == beforeSha && side == Side.LEFT) return lineIndex
|
||||||
|
if (fromCommitSha == afterSha && side == Side.RIGHT) return lineIndex
|
||||||
|
|
||||||
|
return when (side) {
|
||||||
|
Side.LEFT -> {
|
||||||
|
val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha)
|
||||||
|
transferLine(patches, lineIndex, rightToLeft = true).line
|
||||||
|
}
|
||||||
|
Side.RIGHT -> {
|
||||||
|
val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha)
|
||||||
|
transferLine(patches, lineIndex, rightToLeft = false).line
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Map the [lineIndex] in a file in a *commit* [fromCommitSha] to a location in the current [patch]
|
* Map the [lineIndex] in a file in a *commit* [fromCommitSha] to a location in the current [patch]
|
||||||
*
|
*
|
||||||
@@ -39,38 +66,14 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo
|
|||||||
if (fromCommitSha == beforeSha) return DiffLineLocation(Side.LEFT, lineIndex)
|
if (fromCommitSha == beforeSha) return DiffLineLocation(Side.LEFT, lineIndex)
|
||||||
if (fromCommitSha == afterSha) return DiffLineLocation(Side.RIGHT, lineIndex)
|
if (fromCommitSha == afterSha) return DiffLineLocation(Side.RIGHT, lineIndex)
|
||||||
|
|
||||||
fun tryTransferToParent(): DiffLineLocation? {
|
if (!fileHistory.contains(fromCommitSha, patch.filePath)) return null
|
||||||
return if (fileHistory.compare(afterSha, fromCommitSha) < 0) {
|
|
||||||
val patches = fileHistory.getPatchesBetween(afterSha, fromCommitSha)
|
|
||||||
transferLine(patches, lineIndex, true)
|
|
||||||
}
|
|
||||||
else if (fileHistory.compare(beforeSha, fromCommitSha) < 0) {
|
|
||||||
val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha)
|
|
||||||
transferLine(patches, lineIndex, true)
|
|
||||||
}
|
|
||||||
else {
|
|
||||||
null
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fun tryTransferToChild(): DiffLineLocation? {
|
|
||||||
return if (fileHistory.compare(fromCommitSha, beforeSha) < 0) {
|
|
||||||
val patches = fileHistory.getPatchesBetween(fromCommitSha, beforeSha)
|
|
||||||
transferLine(patches, lineIndex, false)
|
|
||||||
}
|
|
||||||
else if (fileHistory.compare(fromCommitSha, afterSha) < 0) {
|
|
||||||
val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha)
|
|
||||||
transferLine(patches, lineIndex, false)
|
|
||||||
}
|
|
||||||
else {
|
|
||||||
null
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return try {
|
return try {
|
||||||
when (bias) {
|
when (bias) {
|
||||||
Side.LEFT -> tryTransferToParent() ?: tryTransferToChild()
|
Side.LEFT -> transferToParent(lineIndex, beforeSha, fromCommitSha, afterSha)
|
||||||
Side.RIGHT -> tryTransferToChild() ?: tryTransferToParent()
|
?: transferToChild(lineIndex, beforeSha, fromCommitSha, afterSha)
|
||||||
|
Side.RIGHT -> transferToChild(lineIndex, beforeSha, fromCommitSha, afterSha)
|
||||||
|
?: transferToParent(lineIndex, beforeSha, fromCommitSha, afterSha)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
catch (e: Exception) {
|
catch (e: Exception) {
|
||||||
@@ -79,9 +82,43 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun transferLine(patchChain: List<TextFilePatch>, lineIndex: Int, rightToLeft: Boolean): DiffLineLocation? {
|
private fun transferToParent(lineIndex: Int, beforeSha: String, fromCommitSha: String, afterSha: String): DiffLineLocation? =
|
||||||
val side: Side = if (rightToLeft) Side.RIGHT else Side.LEFT
|
if (fileHistory.compare(afterSha, fromCommitSha) < 0) {
|
||||||
var currentLine: Int = lineIndex
|
val patches = fileHistory.getPatchesBetween(afterSha, fromCommitSha)
|
||||||
|
transferLine(patches, lineIndex, true).exactLocation()?.let {
|
||||||
|
Side.RIGHT to it
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else if (fileHistory.compare(beforeSha, fromCommitSha) < 0) {
|
||||||
|
val patches = fileHistory.getPatchesBetween(beforeSha, fromCommitSha)
|
||||||
|
transferLine(patches, lineIndex, true).exactLocation()?.let {
|
||||||
|
Side.LEFT to it
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
error("Couldn't find commit ${fromCommitSha}")
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun transferToChild(lineIndex: Int, beforeSha: String, fromCommitSha: String, afterSha: String): DiffLineLocation? =
|
||||||
|
if (fileHistory.compare(fromCommitSha, beforeSha) < 0) {
|
||||||
|
val patches = fileHistory.getPatchesBetween(fromCommitSha, beforeSha)
|
||||||
|
transferLine(patches, lineIndex, false).exactLocation()?.let {
|
||||||
|
Side.LEFT to it
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else if (fileHistory.compare(fromCommitSha, afterSha) < 0) {
|
||||||
|
val patches = fileHistory.getPatchesBetween(fromCommitSha, afterSha)
|
||||||
|
transferLine(patches, lineIndex, false).exactLocation()?.let {
|
||||||
|
Side.RIGHT to it
|
||||||
|
}
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
error("Couldn't find commit ${fromCommitSha}")
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun transferLine(patchChain: List<TextFilePatch>, lineIndex: Int, rightToLeft: Boolean): TransferResult {
|
||||||
|
var currentLine = lineIndex.toDouble()
|
||||||
|
var isEstimate = false
|
||||||
|
|
||||||
val patches = if (rightToLeft) patchChain.asReversed() else patchChain
|
val patches = if (rightToLeft) patchChain.asReversed() else patchChain
|
||||||
|
|
||||||
@@ -91,24 +128,52 @@ class GitTextFilePatchWithHistory(val patch: TextFilePatch, val isCumulative: Bo
|
|||||||
if (rightToLeft) ranges.map { reverseRange(it) } else ranges
|
if (rightToLeft) ranges.map { reverseRange(it) } else ranges
|
||||||
}.flatten()
|
}.flatten()
|
||||||
|
|
||||||
var offset = 0
|
var offset = 0.0
|
||||||
loop@ for (range in changeOnlyRanges) {
|
loop@ for (range in changeOnlyRanges) {
|
||||||
when {
|
when {
|
||||||
currentLine < range.start1 ->
|
currentLine < range.start1 ->
|
||||||
break@loop
|
break@loop
|
||||||
currentLine in range.start1 until range.end1 ->
|
range.start1 <= currentLine && currentLine < range.end1 -> {
|
||||||
return null
|
isEstimate = true
|
||||||
currentLine >= range.end1 ->
|
// Careful when changing: we assume that range.end1 - range.start1 is non-zero due to the above check
|
||||||
|
// Estimated position algo: find the relative position of the line in the hunk, then map it to that relative position in the new hunk
|
||||||
|
// A choice can be made about how to map to 'relative position' though; we choose to find the relative position based on the 'center' of the line within a range:
|
||||||
|
// - in a half-open range [1, 2), line 1 has relative position 0.5 (the 'center' of the line is used rather than the start)
|
||||||
|
// - in a half-open range [1, 3), line 1 has relative position 0.333, line 2 has relative position 0.667
|
||||||
|
val relativePosition = ((currentLine - range.start1) + 1) / ((range.end1 - range.start1) + 1).toDouble()
|
||||||
|
offset -= currentLine - range.start1 // move line to the start of the range in the old hunk
|
||||||
|
// We map relative positions back to the output line in the following way:
|
||||||
|
// - for any relPos and out-range [1, 1) - there no output line -, we map to line 1
|
||||||
|
// - for any relPos and out-range [1, 2) - there is only 1 possible output line -, we map to line 1.5 (and round after all mapping is done)
|
||||||
|
// - for relPos = 0.5 and out-range [1, 3) - there are 2 possible output lines -, we map to line 2
|
||||||
|
// - for relPos = 0.5 and out-range [1, 4) - there are 3 possible output lines -, we map to line 2.5 (and round after all mapping is done)
|
||||||
|
offset += relativePosition * (range.end2 - range.start2) // then to the relative position in the new hunk
|
||||||
|
}
|
||||||
|
range.end1 <= currentLine ->
|
||||||
offset += (range.end2 - range.start2) - (range.end1 - range.start1)
|
offset += (range.end2 - range.start2) - (range.end1 - range.start1)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
currentLine += offset
|
currentLine += offset
|
||||||
}
|
}
|
||||||
return DiffLineLocation(side, currentLine)
|
|
||||||
|
// Note that the only way for the line number to become floating point is when it IS an estimate
|
||||||
|
return when (isEstimate) {
|
||||||
|
false -> TransferResult.ExactTransfer(floor(currentLine).toInt())
|
||||||
|
true -> TransferResult.EstimatedTransfer(floor(currentLine).toInt())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun reverseRange(range: Range) = Range(range.start2, range.end2, range.start1, range.end1)
|
private fun reverseRange(range: Range) = Range(range.start2, range.end2, range.start1, range.end1)
|
||||||
|
|
||||||
|
private sealed interface TransferResult {
|
||||||
|
data class ExactTransfer(override val line: Int) : TransferResult
|
||||||
|
data class EstimatedTransfer(override val line: Int) : TransferResult
|
||||||
|
|
||||||
|
val line: Int?
|
||||||
|
|
||||||
|
fun exactLocation(): Int? = (this as? ExactTransfer)?.line
|
||||||
|
}
|
||||||
|
|
||||||
companion object {
|
companion object {
|
||||||
private val LOG = logger<GitTextFilePatchWithHistory>()
|
private val LOG = logger<GitTextFilePatchWithHistory>()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,273 @@
|
|||||||
|
// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||||
|
package git4idea.changes
|
||||||
|
|
||||||
|
import com.intellij.diff.util.Side
|
||||||
|
import com.intellij.openapi.diff.impl.patch.PatchHunk
|
||||||
|
import com.intellij.openapi.diff.impl.patch.PatchLine
|
||||||
|
import com.intellij.openapi.diff.impl.patch.TextFilePatch
|
||||||
|
import com.intellij.openapi.vcs.FileStatus
|
||||||
|
import org.assertj.core.api.Assertions.assertThat
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource
|
||||||
|
|
||||||
|
class GitTextFilePatchWithHistoryTest {
|
||||||
|
@ParameterizedTest
|
||||||
|
@MethodSource("testCases")
|
||||||
|
fun `forceful mapping works`(case: TestCase) {
|
||||||
|
val patch = createTestPatch(case)
|
||||||
|
assertThat(patch.forcefullyMapLine(case.fromPatch, case.line, case.toSide))
|
||||||
|
.isEqualTo(case.expected)
|
||||||
|
}
|
||||||
|
|
||||||
|
@ParameterizedTest
|
||||||
|
@MethodSource("testCases")
|
||||||
|
fun `regular mapping works`(case: TestCase) {
|
||||||
|
val patch = createTestPatch(case)
|
||||||
|
assertThat(patch.mapLine(case.fromPatch, case.line, case.toSide))
|
||||||
|
.isEqualTo(if (case.isEstimate) case.toSide.other() to case.line // no mapping, just use the easy side TODO: fix with complex start-in-the-middle tests
|
||||||
|
else case.toSide to case.expected)
|
||||||
|
}
|
||||||
|
|
||||||
|
private fun createTestPatch(case : TestCase) = GitTextFilePatchWithHistory(
|
||||||
|
TextFilePatch(Charsets.UTF_8, "\n").apply {
|
||||||
|
beforeVersionId = ROOT_COMMIT
|
||||||
|
afterVersionId = case.patches.last().name
|
||||||
|
|
||||||
|
beforeName = if (case.patches.first().fileStatus == FileStatus.ADDED) null else FILE_NAME
|
||||||
|
afterName = if (case.patches.last().fileStatus == FileStatus.DELETED) null else FILE_NAME
|
||||||
|
},
|
||||||
|
isCumulative = true,
|
||||||
|
fileHistory = object : GitFileHistory {
|
||||||
|
override fun findStartCommit(): String = ROOT_COMMIT
|
||||||
|
override fun findFirstParent(commitSha: String): String? = case.patches.zipWithNext().find { it.second.name == commitSha }?.first?.name
|
||||||
|
override fun contains(commitSha: String, filePath: String): Boolean = commitSha == ROOT_COMMIT || case.patches.any { it.name == commitSha }
|
||||||
|
override fun compare(commitSha1: String, commitSha2: String): Int {
|
||||||
|
val commits = listOf(ROOT_COMMIT) + case.patches.map { it.name }
|
||||||
|
return commits.indexOf(commitSha1) compareTo commits.indexOf(commitSha2)
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun getPatchesBetween(parent: String, child: String): List<TextFilePatch> {
|
||||||
|
val parentIndex = case.patches.indexOfFirst { it.name == parent }
|
||||||
|
val childIndex = case.patches.indexOfFirst { it.name == child }
|
||||||
|
|
||||||
|
val patches = if (childIndex == -1) case.patches.subList(parentIndex + 1, case.patches.size)
|
||||||
|
else case.patches.subList(parentIndex + 1, childIndex + 1)
|
||||||
|
|
||||||
|
return patches.toTextFilePatches()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
//region: Defining patches to test on
|
||||||
|
// just a filler at the start of the patch-list, avoiding the conditions to quickly return the current position
|
||||||
|
val startPatch = TestPatch(
|
||||||
|
name = "start_patch",
|
||||||
|
changes = listOf()
|
||||||
|
)
|
||||||
|
|
||||||
|
val patch1 = TestPatch(
|
||||||
|
name = "patch1",
|
||||||
|
changes = listOf(
|
||||||
|
(0..<0) to (0..<10), // insert 10 lines at line 0
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
val patch2 = TestPatch(
|
||||||
|
name = "patch2",
|
||||||
|
changes = listOf(
|
||||||
|
(1..<4) to (1..<5), // replace lines 1-3 with lines 1-4
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
val patch3 = TestPatch(
|
||||||
|
name = "patch3",
|
||||||
|
changes = listOf(
|
||||||
|
(2..<3) to (2..<4), // replace line 2 with lines 2-3
|
||||||
|
(4..<6) to (5..<5), // delete lines 4-5 (shifted by 1 in result because of the insert at 2-4)
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
// just a filler at the end of the patch-list, avoiding the conditions to quickly return the current position
|
||||||
|
val endPatch = TestPatch(
|
||||||
|
name = "end_patch",
|
||||||
|
changes = listOf()
|
||||||
|
)
|
||||||
|
//endregion
|
||||||
|
|
||||||
|
//region: Defining test cases
|
||||||
|
@JvmStatic
|
||||||
|
fun testCases(): Array<TestCase> = cases {
|
||||||
|
// The first commit pushes all lines forward by 10, other patches shift the line +1, then -1
|
||||||
|
withPatches(patch1, patch2, patch3) {
|
||||||
|
0 `maps to` /* -> 10 -> 11 -> */ 10
|
||||||
|
1 `maps to` /* -> 11 -> 12 -> */ 11
|
||||||
|
2 `maps to` /* -> 12 -> 13 -> */ 12
|
||||||
|
10 `reverse maps to` 0
|
||||||
|
11 `reverse maps to` 1
|
||||||
|
12 `reverse maps to` 2
|
||||||
|
// in reverse, lines 0-10 collapse to 0-0
|
||||||
|
9 `reverse maps to` 0.approximately
|
||||||
|
5 `reverse maps to` 0.approximately
|
||||||
|
}
|
||||||
|
// Single patch that:
|
||||||
|
// - replaces 2-2 with 2-3
|
||||||
|
// - deletes 4-5
|
||||||
|
withPatches(patch3) {
|
||||||
|
0 `maps to` 0
|
||||||
|
1 `maps to` 1
|
||||||
|
2 `maps to` 3.approximately
|
||||||
|
// insert at line 2 causes shift by 1
|
||||||
|
3 `maps to` 4
|
||||||
|
4 `maps to` 5.approximately // deleted
|
||||||
|
5 `maps to` 5.approximately // also deleted, mapped to the same as 4
|
||||||
|
6 `maps to` 5
|
||||||
|
7 `maps to` 6
|
||||||
|
6 `reverse maps to` 7
|
||||||
|
5 `reverse maps to` 6
|
||||||
|
// gap happens because 4-5 was deleted
|
||||||
|
4 `reverse maps to` 3
|
||||||
|
// approximation happens because 2-2 is replaced by 2-3
|
||||||
|
3 `reverse maps to` 2.approximately
|
||||||
|
2 `reverse maps to` 2.approximately
|
||||||
|
1 `reverse maps to` 1
|
||||||
|
}
|
||||||
|
// Illustrations of approximate mapping:
|
||||||
|
// relative position inside range 0-1 of line 0 = 0.5
|
||||||
|
withPatches(TestPatch("estimation [0,1)->[0,0)", listOf((0..<1) to (0..<0)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,1)->[0,1)", listOf((0..<1) to (0..<1)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,1)->[0,2)", listOf((0..<1) to (0..<2)))) {
|
||||||
|
0 `maps to` 1.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,1)->[0,3)", listOf((0..<1) to (0..<3)))) {
|
||||||
|
0 `maps to` 1.approximately
|
||||||
|
}
|
||||||
|
// relative position inside range 0-2 of line 0 = 0.333, 1 = 0.667
|
||||||
|
withPatches(TestPatch("estimation [0,2)->[0,0)", listOf((0..<2) to (0..<0)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
1 `maps to` 0.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,2)->[0,1)", listOf((0..<2) to (0..<1)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
1 `maps to` 0.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,2)->[0,2)", listOf((0..<2) to (0..<2)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
1 `maps to` 1.approximately
|
||||||
|
}
|
||||||
|
withPatches(TestPatch("estimation [0,2)->[0,3)", listOf((0..<2) to (0..<3)))) {
|
||||||
|
0 `maps to` 1.approximately // 0.333 -> line 1 in [0,1,2]
|
||||||
|
1 `maps to` 2.approximately // 0.667 -> line 2 in [0,1,2]
|
||||||
|
}
|
||||||
|
// delete content is just always mapped to the first line
|
||||||
|
withPatches(TestPatch("estimation [0,100)->[0,0)", listOf((0..<100) to (0..<0)))) {
|
||||||
|
0 `maps to` 0.approximately
|
||||||
|
10 `maps to` 0.approximately
|
||||||
|
55 `maps to` 0.approximately
|
||||||
|
99 `maps to` 0.approximately
|
||||||
|
}
|
||||||
|
}
|
||||||
|
//endregion
|
||||||
|
|
||||||
|
//region: Utilities
|
||||||
|
data class TestPatch(
|
||||||
|
val name: String,
|
||||||
|
val changes: List<Pair<IntRange, IntRange>>,
|
||||||
|
val fileStatus: FileStatus = FileStatus.MODIFIED,
|
||||||
|
)
|
||||||
|
|
||||||
|
fun List<TestPatch>.toTextFilePatches(): List<TextFilePatch> {
|
||||||
|
val parentName = ROOT_COMMIT
|
||||||
|
|
||||||
|
val patches = mutableListOf<TextFilePatch>()
|
||||||
|
for (patch in this) {
|
||||||
|
patches.add(TextFilePatch(Charsets.UTF_8, "\n").apply {
|
||||||
|
if (patch.fileStatus != FileStatus.ADDED) {
|
||||||
|
beforeVersionId = parentName
|
||||||
|
beforeName = FILE_NAME
|
||||||
|
}
|
||||||
|
|
||||||
|
if (patch.fileStatus != FileStatus.DELETED) {
|
||||||
|
afterVersionId = patch.name
|
||||||
|
afterName = FILE_NAME
|
||||||
|
}
|
||||||
|
|
||||||
|
for (change in patch.changes) {
|
||||||
|
addHunk(PatchHunk(change.first.first, change.first.last, change.second.first, change.second.last).apply {
|
||||||
|
// our patch hunks are simple: no context lines, just changes
|
||||||
|
repeat((change.first.last - change.first.first) + 1) { addLine(REMOVE_LINE) }
|
||||||
|
repeat((change.second.last - change.second.first) + 1) { addLine(ADD_LINE) }
|
||||||
|
})
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
return patches.toList()
|
||||||
|
}
|
||||||
|
|
||||||
|
data class TestCase(
|
||||||
|
val patches: List<TestPatch>,
|
||||||
|
val fromPatch: String,
|
||||||
|
val line: Int,
|
||||||
|
val expected: Int,
|
||||||
|
val isEstimate: Boolean,
|
||||||
|
val toSide: Side,
|
||||||
|
) {
|
||||||
|
private val filteredPatches = patches.filter { it.name != startPatch.name && it.name != endPatch.name }
|
||||||
|
|
||||||
|
override fun toString(): String =
|
||||||
|
"[${filteredPatches.joinToString { it.name }}]: " +
|
||||||
|
"$line${if (isEstimate) " approximately" else ""}${if (Side.LEFT == toSide) " reverse" else ""} maps to ${expected}"
|
||||||
|
}
|
||||||
|
|
||||||
|
@JvmInline
|
||||||
|
value class Approximately(val line: Int)
|
||||||
|
|
||||||
|
class TestCaseBuilder(
|
||||||
|
private val actualPatches: List<TestPatch> = listOf(),
|
||||||
|
private val fromPatch: String? = null,
|
||||||
|
) {
|
||||||
|
private val firstCommit get() = startPatch.name
|
||||||
|
private val lastCommit get() = actualPatches.last().name
|
||||||
|
|
||||||
|
// surrounded with a start and end so that we trick the algo into actually executing
|
||||||
|
// without a start and end patch the algo will just shortcut to return the current line index
|
||||||
|
private val surroundedPatches = listOf(startPatch) + actualPatches + listOf(endPatch)
|
||||||
|
var cases = mutableListOf<TestCase>()
|
||||||
|
|
||||||
|
private fun register(case: TestCase) {
|
||||||
|
cases.add(case)
|
||||||
|
}
|
||||||
|
|
||||||
|
fun withPatches(vararg patches: TestPatch, block: TestCaseBuilder.() -> Unit) {
|
||||||
|
cases += TestCaseBuilder(patches.toList()).apply { block() }.cases
|
||||||
|
}
|
||||||
|
|
||||||
|
fun fromPatch(patch: String, block: TestCaseBuilder.() -> Unit) {
|
||||||
|
cases += TestCaseBuilder(surroundedPatches, patch).apply { block() }.cases
|
||||||
|
}
|
||||||
|
|
||||||
|
//@formatter:off
|
||||||
|
infix fun Int.`maps to`(line: Int) = register(TestCase(surroundedPatches, fromPatch ?: firstCommit, this, line, false, Side.RIGHT))
|
||||||
|
infix fun Int.`reverse maps to`(line: Int) = register(TestCase(surroundedPatches, fromPatch ?: lastCommit, this, line, false, Side.LEFT))
|
||||||
|
|
||||||
|
val Int.approximately get() = Approximately(this)
|
||||||
|
infix fun Int.`maps to`(approximately: Approximately) = register(TestCase(surroundedPatches, fromPatch ?: firstCommit, this, approximately.line, true, Side.RIGHT))
|
||||||
|
infix fun Int.`reverse maps to`(approximately: Approximately) = register(TestCase(surroundedPatches, fromPatch ?: lastCommit, this, approximately.line, true, Side.LEFT))
|
||||||
|
//@formatter:on
|
||||||
|
}
|
||||||
|
|
||||||
|
fun cases(block: TestCaseBuilder.() -> Unit): Array<TestCase> =
|
||||||
|
TestCaseBuilder().apply { block() }.cases.toTypedArray()
|
||||||
|
|
||||||
|
val REMOVE_LINE = PatchLine(PatchLine.Type.REMOVE, "").apply { isSuppressNewLine = true }
|
||||||
|
val ADD_LINE = PatchLine(PatchLine.Type.ADD, "").apply { isSuppressNewLine = true }
|
||||||
|
|
||||||
|
const val ROOT_COMMIT = "root"
|
||||||
|
const val FILE_NAME = "file"
|
||||||
|
//endregion
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -15,22 +15,23 @@ import org.jetbrains.plugins.github.api.data.GHNode
|
|||||||
import java.util.*
|
import java.util.*
|
||||||
|
|
||||||
@GraphQLFragment("/graphql/fragment/pullRequestReviewThread.graphql")
|
@GraphQLFragment("/graphql/fragment/pullRequestReviewThread.graphql")
|
||||||
data class GHPullRequestReviewThread(override val id: String,
|
data class GHPullRequestReviewThread(
|
||||||
val isResolved: Boolean,
|
override val id: String,
|
||||||
val isOutdated: Boolean,
|
val isResolved: Boolean,
|
||||||
val path: String,
|
val isOutdated: Boolean,
|
||||||
@JsonProperty("diffSide") val side: Side,
|
val path: String,
|
||||||
val line: Int?,
|
@JsonProperty("diffSide") val side: Side,
|
||||||
val originalLine: Int?,
|
val line: Int?,
|
||||||
@JsonProperty("startDiffSide") val startSide: Side?,
|
val originalLine: Int?,
|
||||||
val startLine: Int?,
|
@JsonProperty("startDiffSide") val startSide: Side?,
|
||||||
val originalStartLine: Int?,
|
val startLine: Int?,
|
||||||
// To be precise: the elements of this list can be null, but should practically never be...
|
val originalStartLine: Int?,
|
||||||
@JsonProperty("comments") private val commentsNodes: GraphQLNodesDTO<GHPullRequestReviewComment>,
|
// To be precise: the elements of this list can be null, but should practically never be...
|
||||||
val viewerCanReply: Boolean,
|
@JsonProperty("comments") private val commentsNodes: GraphQLNodesDTO<GHPullRequestReviewComment>,
|
||||||
val viewerCanResolve: Boolean,
|
val viewerCanReply: Boolean,
|
||||||
val viewerCanUnresolve: Boolean)
|
val viewerCanResolve: Boolean,
|
||||||
: GHNode(id) {
|
val viewerCanUnresolve: Boolean,
|
||||||
|
) : GHNode(id) {
|
||||||
@JsonIgnore
|
@JsonIgnore
|
||||||
val comments: List<GHPullRequestReviewComment> = commentsNodes.nodes
|
val comments: List<GHPullRequestReviewComment> = commentsNodes.nodes
|
||||||
@JsonIgnore
|
@JsonIgnore
|
||||||
@@ -59,39 +60,48 @@ fun GHPullRequestReviewThread.isVisible(viewOption: DiscussionsViewOption): Bool
|
|||||||
DiscussionsViewOption.DONT_SHOW -> false
|
DiscussionsViewOption.DONT_SHOW -> false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fun GHPullRequestReviewThread.mapToLeftSideLine(diffData: GitTextFilePatchWithHistory): Int? =
|
||||||
|
mapToSidedLine(diffData, Side.LEFT)
|
||||||
|
|
||||||
|
fun GHPullRequestReviewThread.mapToRightSideLine(diffData: GitTextFilePatchWithHistory): Int? =
|
||||||
|
mapToSidedLine(diffData, Side.RIGHT)
|
||||||
|
|
||||||
|
private fun GHPullRequestReviewThread.mapToSidedLine(diffData: GitTextFilePatchWithHistory, side: Side): Int? {
|
||||||
|
val threadData = this
|
||||||
|
if (threadData.line == null && threadData.originalLine == null) return null
|
||||||
|
|
||||||
|
val lineIndex = threadData.line ?: threadData.originalLine ?: return null
|
||||||
|
val fromCommitSha = fromCommitSha(diffData) ?: return null
|
||||||
|
|
||||||
|
return diffData.forcefullyMapLine(fromCommitSha, lineIndex - 1, side)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO: Write tests to illustrate and check the working of location mapping :'(
|
||||||
fun GHPullRequestReviewThread.mapToLocation(diffData: GitTextFilePatchWithHistory, sideBias: Side? = null): DiffLineLocation? {
|
fun GHPullRequestReviewThread.mapToLocation(diffData: GitTextFilePatchWithHistory, sideBias: Side? = null): DiffLineLocation? {
|
||||||
val threadData = this
|
val threadData = this
|
||||||
if (threadData.line == null && threadData.originalLine == null) return null
|
if (threadData.line == null && threadData.originalLine == null) return null
|
||||||
|
|
||||||
return if (threadData.line != null) {
|
val lineIndex = threadData.line ?: threadData.originalLine ?: return null
|
||||||
val commitSha = threadData.commit?.oid ?: return null
|
val fromCommitSha = fromCommitSha(diffData) ?: return null
|
||||||
if (!diffData.contains(commitSha, threadData.path)) return null
|
|
||||||
when (threadData.side) {
|
val sideBias = sideBias ?: threadData.side
|
||||||
Side.RIGHT -> {
|
|
||||||
diffData.mapLine(commitSha, threadData.line - 1, sideBias ?: Side.RIGHT)
|
return diffData.mapLine(fromCommitSha, lineIndex - 1, sideBias)
|
||||||
}
|
}
|
||||||
Side.LEFT -> {
|
|
||||||
diffData.fileHistory.findStartCommit()?.let { baseSha ->
|
private fun GHPullRequestReviewThread.fromCommitSha(diffData: GitTextFilePatchWithHistory): String? {
|
||||||
diffData.mapLine(baseSha, threadData.line - 1, sideBias ?: Side.LEFT)
|
val threadData = this
|
||||||
}
|
|
||||||
}
|
return if (threadData.line != null) when (threadData.side) {
|
||||||
}
|
Side.RIGHT -> threadData.commit?.oid
|
||||||
|
Side.LEFT -> diffData.fileHistory.findStartCommit()
|
||||||
}
|
}
|
||||||
else if (threadData.originalLine != null) {
|
else if (threadData.originalLine != null) {
|
||||||
val originalCommitSha = threadData.originalCommit?.oid ?: return null
|
val originalCommitSha = threadData.originalCommit?.oid ?: return null
|
||||||
if (!diffData.contains(originalCommitSha, threadData.path)) return null
|
|
||||||
when (threadData.side) {
|
when (threadData.side) {
|
||||||
Side.RIGHT -> {
|
Side.RIGHT -> originalCommitSha
|
||||||
diffData.mapLine(originalCommitSha, threadData.originalLine - 1, sideBias ?: Side.RIGHT)
|
Side.LEFT -> diffData.fileHistory.findFirstParent(originalCommitSha)
|
||||||
}
|
|
||||||
Side.LEFT -> {
|
|
||||||
diffData.fileHistory.findFirstParent(originalCommitSha)?.let { parentSha ->
|
|
||||||
diffData.mapLine(parentSha, threadData.originalLine - 1, sideBias ?: Side.LEFT)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
else {
|
else null
|
||||||
null
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user