Fixes #103 by partitioning important and non-important changes

Author: CodesAway

BEXCodeCompare/src/test/java/info/codesaway/bex/diff/DiffHelperTests.java

@@ -30,6 +30,7 @@
 import info.codesaway.bex.diff.myers.MyersLinearDiff;
 import info.codesaway.bex.diff.patience.PatienceDiff;
 import info.codesaway.bex.diff.patience.PatienceMatch;
+import info.codesaway.bex.diff.substitution.SubstitutionDiffTypeValue;
 
 class DiffHelperTests {
 
@@ -165,6 +166,34 @@ public void testNeverCombineToDiffBlocks() {
 		assertEquals(expected, diffUnits);
 	}
 
+	@Test
+	// Issue #103
+	public void testCombineToDiffBlocksPartitionImportantAndNonImportantChanges() {
+		// Text doesn't matter, since combine focuses on DiffType
+		List<DiffLine> leftLines = ImmutableList.of(new DiffLine(0, ""), new DiffLine(1, ""), new DiffLine(2, ""),
+				new DiffLine(3, ""));
+		List<DiffLine> rightLines = ImmutableList.of(new DiffLine(0, ""), new DiffLine(1, ""), new DiffLine(2, ""),
+				new DiffLine(3, ""));
+
+		DiffType shouldTreatAsNormalizedEqual = new SubstitutionDiffTypeValue(' ', "", false, true);
+
+		DiffEdit first = new DiffEdit(INSERT, leftLines.get(0), rightLines.get(0));
+		DiffEdit second = new DiffEdit(shouldTreatAsNormalizedEqual, leftLines.get(1), rightLines.get(1));
+		DiffEdit third = new DiffEdit(shouldTreatAsNormalizedEqual, leftLines.get(2), rightLines.get(2));
+		DiffEdit fourth = new DiffEdit(INSERT, leftLines.get(3), rightLines.get(3));
+
+		List<DiffEdit> diff = ImmutableList.of(first, second, third, fourth);
+
+		List<DiffUnit> diffUnits = DiffHelper.combineToDiffBlocks(diff, true,
+				(x, y) -> x.shouldTreatAsNormalizedEqual() == y.shouldTreatAsNormalizedEqual());
+
+		List<DiffUnit> expected = ImmutableList.of(first,
+				new DiffBlock(shouldTreatAsNormalizedEqual, diff.subList(1, 3)),
+				fourth);
+
+		assertEquals(expected, diffUnits);
+	}
+
 	@Test
 	public void testDetermineEnclosedRange() {
 		List<DiffLine> leftLines = ImmutableList.of(new DiffLine(0, "a"), new DiffLine(1, "b"), new DiffLine(2, "c"));

BEXCodeCompare/src/test/java/info/codesaway/bex/matching/BEXParseJSPTest.java

@@ -2,8 +2,10 @@
 
 import static info.codesaway.bex.IntBEXRange.closed;
 import static info.codesaway.bex.IntBEXRange.closedOpen;
+import static info.codesaway.bex.IntBEXRange.singleton;
 import static info.codesaway.bex.matching.BEXMatchingStateOption.IN_EXPRESSION_BLOCK;
 import static info.codesaway.bex.matching.BEXMatchingStateOption.IN_STRING_LITERAL;
+import static info.codesaway.bex.matching.BEXMatchingStateOption.IN_TAG;
 import static info.codesaway.bex.util.BEXUtilities.entry;
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -16,10 +18,14 @@ void testExpressionEndsThenAnotherImmediatelyStarts() {
 		BEXString bexString = new BEXString(text, BEXMatchingLanguage.JSP);
 
 		assertThat(bexString.getTextStateMap().asMapOfRanges())
-				.containsExactly(entry(closedOpen(8, 9), IN_STRING_LITERAL),
+				.containsExactly(
+						entry(closedOpen(0, 8), IN_TAG),
+						entry(closedOpen(8, 9), IN_STRING_LITERAL),
 						entry(closed(9, 20), IN_EXPRESSION_BLOCK),
 						entry(closed(21, 33), IN_EXPRESSION_BLOCK),
-						entry(closed(34, 34), IN_STRING_LITERAL),
-						entry(closed(36, 47), IN_EXPRESSION_BLOCK));
+						entry(singleton(34), IN_STRING_LITERAL),
+						entry(singleton(35), IN_TAG),
+						entry(closed(36, 47), IN_EXPRESSION_BLOCK),
+						entry(closed(48, 51), IN_TAG));
 	}
 }

BEXCodeComparePlugin/src/info/codesaway/bex/compare/RangeComparatorBEX.java

@@ -234,7 +234,11 @@ private void computeDifferences(final SubMonitor newChild, final boolean updateV
 		// (if true, okay to combine using existing logic)
 		// (if false, should never combine, even if current logic would allow)
 		// This way, can ensure we don't combine unimportant refactorings in the same change as important changes
-		List<DiffUnit> diffBlocks = DiffHelper.combineToDiffBlocks(diff, true);
+		List<DiffUnit> diffBlocks = DiffHelper.combineToDiffBlocks(diff, true,
+				// Fixes issue #103 - only combine if both diffs either should or should not
+				// (what was happening was it was combining non-important changes with important changes)
+				// (this caused Eclipse to show more differences than expected)
+				(x, y) -> x.shouldTreatAsNormalizedEqual() == y.shouldTreatAsNormalizedEqual());
 		//		List<DiffUnit> diffBlocks = DiffHelper.combineToDiffBlocks(diff, false);
 
 		// Handle ignoring blank lines or comments (if option is enabled)