MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

merge: use non-greedy approach for "magic" conflict resolve

Browse Source 
* it produces lots of confusing false-positive results
* move "greedy" approach under registry key
This commit is contained in:
Aleksey Pivovarov
2016-10-17 17:13:09 +03:00
parent 2485cd2b47
commit 3fb83747e1
5 changed files with 129 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
platform/diff-impl/src/com/intellij/diff/comparison/ComparisonMergeUtil.java
+7 -1
View File
@@ -20,6 +20,7 @@ import com.intellij.diff.util.MergeRange;
import com.intellij.diff.util.Range;
import com.intellij.diff.util.Side;
import com.intellij.openapi.progress.ProgressIndicator;
import com.intellij.openapi.util.registry.Registry;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -160,6 +161,11 @@ public class ComparisonMergeUtil {
public static CharSequence tryResolveConflict(@NotNull CharSequence leftText,
@NotNull CharSequence baseText,
@NotNull CharSequence rightText) {
return MergeResolveUtil.tryResolveConflict(leftText, baseText, rightText);
if (Registry.is("diff.merge.resolve.conflict.action.use.greedy.approach")) {
return MergeResolveUtil.tryGreedyResolve(leftText, baseText, rightText);
}
else {
return MergeResolveUtil.tryResolve(leftText, baseText, rightText);
}
}
}
platform/diff-impl/src/com/intellij/diff/comparison/MergeResolveUtil.kt
+59 -3
View File
@@ -16,13 +16,29 @@
package com.intellij.diff.comparison
import com.intellij.diff.fragments.DiffFragment
import com.intellij.diff.util.DiffUtil
import com.intellij.diff.util.Side
import com.intellij.diff.util.Side.LEFT
import com.intellij.diff.util.Side.RIGHT
import com.intellij.diff.util.TextDiffType
import com.intellij.diff.util.ThreeSide
import com.intellij.openapi.progress.DumbProgressIndicator
import com.intellij.util.text.MergingCharSequence
object MergeResolveUtil {
@JvmStatic
fun tryResolve(leftText: CharSequence, baseText: CharSequence, rightText: CharSequence): CharSequence? {
try {
val resolved = tryResolve(leftText, baseText, rightText, ComparisonPolicy.DEFAULT)
if (resolved != null) return resolved
return tryResolve(leftText, baseText, rightText, ComparisonPolicy.IGNORE_WHITESPACES)
}
catch (e: DiffTooBigException) {
return null
}
}
/*
* Here we assume, that resolve results are explicitly verified by user and can be safely undone.
* Thus we trade higher chances of incorrect resolve for higher chances of correct resolve.
@@ -36,12 +52,12 @@ object MergeResolveUtil {
* modifications can be considered as "insertion + deletion" and resolved accordingly.
*/
@JvmStatic
fun tryResolveConflict(leftText: CharSequence, baseText: CharSequence, rightText: CharSequence): CharSequence? {
fun tryGreedyResolve(leftText: CharSequence, baseText: CharSequence, rightText: CharSequence): CharSequence? {
try {
val resolved = Helper(leftText, baseText, rightText).execute(ComparisonPolicy.DEFAULT)
val resolved = tryGreedyResolve(leftText, baseText, rightText, ComparisonPolicy.DEFAULT)
if (resolved != null) return resolved
return Helper(leftText, baseText, rightText).execute(ComparisonPolicy.IGNORE_WHITESPACES)
return tryGreedyResolve(leftText, baseText, rightText, ComparisonPolicy.IGNORE_WHITESPACES)
}
catch (e: DiffTooBigException) {
return null
@@ -49,6 +65,46 @@ object MergeResolveUtil {
}
}
private fun tryResolve(leftText: CharSequence, baseText: CharSequence, rightText: CharSequence,
policy: ComparisonPolicy): CharSequence? {
val texts = listOf(leftText, baseText, rightText)
val changes = ByWord.compare(leftText, baseText, rightText, policy, DumbProgressIndicator.INSTANCE)
val newContent = StringBuilder()
var last = 0
for (fragment in changes) {
val type = DiffUtil.getWordMergeType(fragment, texts, policy)
if (type.diffType == TextDiffType.CONFLICT) return null;
val baseStart = fragment.getStartOffset(ThreeSide.BASE)
val baseEnd = fragment.getEndOffset(ThreeSide.BASE)
newContent.append(baseText, last, baseStart)
if (type.isChange(Side.LEFT)) {
val leftStart = fragment.getStartOffset(ThreeSide.LEFT)
val leftEnd = fragment.getEndOffset(ThreeSide.LEFT)
newContent.append(leftText, leftStart, leftEnd)
}
else {
val rightStart = fragment.getStartOffset(ThreeSide.RIGHT)
val rightEnd = fragment.getEndOffset(ThreeSide.RIGHT)
newContent.append(rightText, rightStart, rightEnd)
}
last = baseEnd
}
newContent.append(baseText, last, baseText.length)
return newContent.toString()
}
private fun tryGreedyResolve(leftText: CharSequence, baseText: CharSequence, rightText: CharSequence,
policy: ComparisonPolicy): CharSequence? {
return Helper(leftText, baseText, rightText).execute(policy)
}
private class Helper(val leftText: CharSequence, val baseText: CharSequence, val rightText: CharSequence) {
val newContent = StringBuilder()
platform/diff-impl/src/com/intellij/diff/util/DiffUtil.java
+32
View File
@@ -23,6 +23,7 @@ import com.intellij.diff.SuppressiveDiffTool;
import com.intellij.diff.comparison.ByWord;
import com.intellij.diff.comparison.ComparisonManager;
import com.intellij.diff.comparison.ComparisonPolicy;
import com.intellij.diff.comparison.ComparisonUtil;
import com.intellij.diff.contents.DiffContent;
import com.intellij.diff.contents.DocumentContent;
import com.intellij.diff.contents.EmptyContent;
@@ -30,6 +31,7 @@ import com.intellij.diff.contents.FileContent;
import com.intellij.diff.fragments.DiffFragment;
import com.intellij.diff.fragments.LineFragment;
import com.intellij.diff.fragments.MergeLineFragment;
import com.intellij.diff.fragments.MergeWordFragment;
import com.intellij.diff.impl.DiffSettingsHolder;
import com.intellij.diff.impl.DiffSettingsHolder.DiffSettings;
import com.intellij.diff.requests.ContentDiffRequest;
@@ -1069,6 +1071,36 @@ public class DiffUtil {
return fragment.getStartLine(side) == fragment.getEndLine(side);
}
@NotNull
public static MergeConflictType getWordMergeType(@NotNull MergeWordFragment fragment,
@NotNull List<? extends CharSequence> texts,
@NotNull ComparisonPolicy policy) {
return getMergeType((side) -> isWordMergeIntervalEmpty(fragment, side),
(side1, side2) -> compareWordMergeContents(fragment, texts, policy, side1, side2));
}
private static boolean compareWordMergeContents(@NotNull MergeWordFragment fragment,
@NotNull List<? extends CharSequence> texts,
@NotNull ComparisonPolicy policy,
@NotNull ThreeSide side1,
@NotNull ThreeSide side2) {
int start1 = fragment.getStartOffset(side1);
int end1 = fragment.getEndOffset(side1);
int start2 = fragment.getStartOffset(side2);
int end2 = fragment.getEndOffset(side2);
CharSequence document1 = side1.select(texts);
CharSequence document2 = side2.select(texts);
CharSequence content1 = document1.subSequence(start1, end1);
CharSequence content2 = document2.subSequence(start2, end2);
return ComparisonUtil.isEquals(content1, content2, policy);
}
private static boolean isWordMergeIntervalEmpty(@NotNull MergeWordFragment fragment, @NotNull ThreeSide side) {
return fragment.getStartOffset(side) == fragment.getEndOffset(side);
}
//
// Writable
//
platform/diff-impl/tests/com/intellij/diff/comparison/MergeResolveUtilTest.kt
+29 -12
View File
@@ -118,35 +118,35 @@ class MergeResolveUtilTest : DiffTestCase() {
}
fun testNonFailureConflicts() {
test(
testGreedy(
"x X x",
"x x",
"x X Y x",
"x Y x"
)
test(
testGreedy(
"x X x",
"x x",
"x Y X x",
"x Y x"
)
test(
testGreedy(
"x X Y x",
"x X x",
"x Y x",
"x x"
)
test(
testGreedy(
"x X Y Z x",
"x X x",
"x Z x",
"x x"
)
test(
testGreedy(
"x A B C D E F G H K x",
"x C F K x",
"x A D H x",
@@ -157,21 +157,21 @@ class MergeResolveUtilTest : DiffTestCase() {
fun testConfusingConflicts() {
// these cases might be a failure as well
test(
testGreedy(
"x X x",
"x x",
"x Z x",
"xZ x"
)
test(
testGreedy(
"x X X x",
"x X Y X x",
"x x",
"x Y x"
)
test(
testGreedy(
"x X x",
"x x",
"x Y x",
@@ -179,7 +179,7 @@ class MergeResolveUtilTest : DiffTestCase() {
)
test(
testGreedy(
"x X X x",
"x Y x",
"x X Y x",
@@ -187,8 +187,25 @@ class MergeResolveUtilTest : DiffTestCase() {
)
}
private fun test(base: String, left: String, right: String, expected: String?) {
val actual = MergeResolveUtil.tryResolveConflict(left, base, right)
assertEquals(expected, actual?.toString())
private fun testGreedy(base: String, left: String, right: String, expected: String?) {
test(base, left, right, expected, true);
}
private fun test(base: String, left: String, right: String, expected: String?, isGreedy: Boolean = false) {
val simpleResult = MergeResolveUtil.tryResolve(left, base, right)
val magicResult = MergeResolveUtil.tryGreedyResolve(left, base, right);
if (expected == null) {
assertNull(simpleResult)
assertNull(magicResult)
}
else if (isGreedy) {
assertNull(simpleResult)
assertEquals(expected, magicResult)
}
else {
assertEquals(expected, simpleResult)
assertEquals(expected, magicResult)
}
}
}
platform/util/resources/misc/registry.properties
+2
View File
@@ -491,6 +491,8 @@ diff.divider.repainting.disable.blitting=true
diff.divider.repainting.disable.blitting.description=Fix painting glitch on scrolling in diff - disable BLIT_SCROLL_MODE to force repainting with RepaintManager
diff.merge.resolve.conflict.action.visible=true
diff.merge.resolve.conflict.action.visible.description=Allows to resolve some conflict in merge in one click (with a high probability of wrong result)
diff.merge.resolve.conflict.action.use.greedy.approach=false
diff.merge.resolve.conflict.action.use.greedy.approach.description=Use greedy heuristic in attempt to resolve conflict. This leads to higher amounts of false-positive and true-positive results.
diff.enable.psi.highlighting=true
diff.enable.psi.highlighting.description=Enable advanced highlighting and code navigation in VCS content in diff viewers.
diff.pass.rich.editor.context=false