Add heuristic to better highlight block reindents
If the leading indentation is being changed on a block, it can
easily show up as a series of consecutive edits where there is
only one line of gap between each edit, due to these lines being
either blank lines, or relatively trivial contexts like "\t}\n"
to close a code block.
Combine the edits together into a single edit before we enter the
intraline difference algorithm. This will permit us to instead see
the whitespace introduction or removal at the start of the line(s)
affected, while retaining context through the rest of the region.
The heuristic assumes a C/C++/Java/Objective-C/JavaScript style of
language where "}" or "/*" on a line by itself would be common as
meaningless context in a multi-line region. We also try to catch
really common control keywords, like "try {" or "} finally {" that
appear on a line by themselves by looking for these cases which end
with the opening brace. To also try to cater to the Python style
of language families, we also permit ":" on the end of the line.
This partially improves the nasty rewrite situation identified in
issue 245, so I'm tagging this commit as though it fixed the bug,
as in general even rewrites look better due to the insanely common
"\s+}\n" tokens being collapsed into the region.
Bug: issue 245
Change-Id: Ie9f4210c2618ac0859e1087c54bd65bc4595495a
Signed-off-by: Shawn O. Pearce <sop@google.com>
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
index 090c39f..e05faba 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
@@ -55,6 +55,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
+import java.util.regex.Pattern;
/** Provides a cached list of {@link PatchListEntry}. */
@Singleton
@@ -62,6 +63,11 @@
private static final String CACHE_NAME = "diff";
private static final boolean dynamic = false;
+ private static final Pattern BLANK_LINE_RE =
+ Pattern.compile("^[ \\t]*(|[{}]|/\\*\\*?|\\*)[ \\t]*$");
+ private static final Pattern CONTROL_BLOCK_START_RE =
+ Pattern.compile("[{:][ \\t]*$");
+
public static Module module() {
return new CacheModule() {
@Override
@@ -231,6 +237,9 @@
edits = new ArrayList<Edit>(edits);
aContent = read(repo, fileHeader.getOldName(), aTree);
bContent = read(repo, fileHeader.getNewName(), bTree);
+ combineLineEdits(edits, aContent, bContent);
+ i = -1; // restart the entire scan after combining lines.
+ continue;
}
CharText a = new CharText(aContent, e.getBeginA(), e.getEndA());
@@ -378,6 +387,51 @@
return new PatchListEntry(fileHeader, edits);
}
+ private static void combineLineEdits(List<Edit> edits, Text a, Text b) {
+ for (int j = 0; j < edits.size() - 1;) {
+ Edit c = edits.get(j);
+ Edit n = edits.get(j + 1);
+
+ // Combine edits that are really close together. Right now our rule
+ // is, coalesce two line edits which are only one line apart if that
+ // common context line is either a "pointless line", or is identical
+ // on both sides and starts a new block of code. These are mostly
+ // block reindents to add or remove control flow operators.
+ //
+ final int ad = n.getBeginA() - c.getEndA();
+ final int bd = n.getBeginB() - c.getEndB();
+ if ((1 <= ad && isBlankLineGap(a, c.getEndA(), n.getBeginA()))
+ || (1 <= bd && isBlankLineGap(b, c.getEndB(), n.getBeginB()))
+ || (ad == 1 && bd == 1 && isControlBlockStart(a, c.getEndA()))) {
+ int ab = c.getBeginA();
+ int ae = n.getEndA();
+
+ int bb = c.getBeginB();
+ int be = n.getEndB();
+
+ edits.set(j, new Edit(ab, ae, bb, be));
+ edits.remove(j + 1);
+ continue;
+ }
+
+ j++;
+ }
+ }
+
+ private static boolean isBlankLineGap(Text a, int b, int e) {
+ for (; b < e; b++) {
+ if (!BLANK_LINE_RE.matcher(a.getLine(b)).matches()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ private static boolean isControlBlockStart(Text a, int idx) {
+ final String l = a.getLine(idx);
+ return CONTROL_BLOCK_START_RE.matcher(l).find();
+ }
+
private static boolean canCoalesce(CharText a, int b, int e) {
while (b < e) {
if (a.charAt(b++) == '\n') {
