opendev/gerrit
Code Issues Proposed changes

Highlight line-level (aka word) differences in files

Browse Source 
We now highlight any changed words within a line replace edit,
making the actual changes stand out against the surrounding context
that makes up the line.

The highlight is computed by constructing a string that covers the
entire replaced region and then running the Myers diff algorithm
over the individual characters of those two regions.

To avoid tiny edits interleaved at every other character in a
sentance we combine two neighboring character edits together if
there are only 1 or 2 characters between them.  There are probably
many ways to improve on this algorithm to avoid some nasty corner
display cases, but this rule is good enough for now.

The highlight data is computed and stored as part of the diff cache,
which requires a schema change in this commit.  So existing diff
cache records will be flushed on the next server start, and they
will be recomputed on demand.

Bug: issue 169
Change-Id: I69142ebef600e8c3c65821272dad3ee04a497654
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2010-01-30 19:41:17 -08:00
parent 0e4f33b82f
commit 7dafe19aee
14 changed files with 482 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
gerrit-server/src/main/java/com/google/gerrit/server/patch/CharText.java Normal file
View File

@@ -0,0 +1,35 @@
// Copyright (C) 2010 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.patch;
import org.eclipse.jgit.diff.Sequence;
class CharText implements Sequence {
private final String content;
CharText(Text text, int s, int e) {
content = text.getLines(s, e);
}
@Override
public boolean equals(int a, Sequence other, int b) {
return content.charAt(a) == ((CharText) other).content.charAt(b);
}
@Override
public int size() {
return content.length();
}
}

95
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListCacheImpl.java
View File

@@ -31,18 +31,28 @@ import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.diff.MyersDiff;
import org.eclipse.jgit.diff.ReplaceEdit;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectLoader;
import org.eclipse.jgit.lib.ObjectWriter;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.patch.FileHeader;
import org.eclipse.jgit.patch.FileHeader.PatchType;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevTree;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.treewalk.TreeWalk;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
/** Provides a cached list of {@link PatchListEntry}. */
@@ -109,7 +119,8 @@ public class PatchListCacheImpl implements PatchListCache {
private PatchList readPatchList(final PatchListKey key, final Repository repo)
throws IOException {
final RevCommit b = new RevWalk(repo).parseCommit(key.getNewId());
final RevWalk rw = new RevWalk(repo);
final RevCommit b = rw.parseCommit(key.getNewId());
final AnyObjectId a = aFor(key, repo, b);
final List<String> args = new ArrayList<String>();
@@ -163,14 +174,94 @@ public class PatchListCacheImpl implements PatchListCache {
}
}
RevTree aTree = a != null ? rw.parseCommit(a).getTree() : null;
RevTree bTree = b.getTree();
final int cnt = p.getFiles().size();
final PatchListEntry[] entries = new PatchListEntry[cnt];
for (int i = 0; i < cnt; i++) {
entries[i] = new PatchListEntry(p.getFiles().get(i));
entries[i] = newEntry(repo, aTree, bTree, p.getFiles().get(i));
}
return new PatchList(a, b, entries);
}
private static PatchListEntry newEntry(Repository repo, RevTree aTree,
RevTree bTree, FileHeader fileHeader) throws IOException {
if (fileHeader.getHunks().isEmpty()) {
return new PatchListEntry(fileHeader, Collections.<Edit> emptyList());
}
List<Edit> edits = fileHeader.toEditList();
// Bypass the longer task of looking for replacement edits if
// there cannot be a replacement within plain text.
//
if (aTree == null /* want combined diff */) {
return new PatchListEntry(fileHeader, edits);
}
if (fileHeader.getPatchType() != PatchType.UNIFIED || edits.isEmpty()) {
return new PatchListEntry(fileHeader, edits);
}
switch (fileHeader.getChangeType()) {
case ADD:
case DELETE:
return new PatchListEntry(fileHeader, edits);
}
Text aContent = null;
Text bContent = null;
for (int i = 0; i < edits.size(); i++) {
Edit e = edits.get(i);
if (e.getType() == Edit.Type.REPLACE) {
if (aContent == null) {
edits = new ArrayList<Edit>(edits);
aContent = read(repo, fileHeader.getOldName(), aTree);
bContent = read(repo, fileHeader.getNewName(), bTree);
}
CharText a = new CharText(aContent, e.getBeginA(), e.getEndA());
CharText b = new CharText(bContent, e.getBeginB(), e.getEndB());
List<Edit> wordEdits = new MyersDiff(a, b).getEdits();
for (int j = 0; j < wordEdits.size() - 1;) {
Edit c = wordEdits.get(j);
Edit n = wordEdits.get(j + 1);
if (n.getBeginA() - c.getEndA() <= 2
|| n.getBeginB() - c.getEndB() <= 2) {
// This edit is incredibly close to the start of the next.
// Combine them together.
//
wordEdits.set(j, new Edit(c.getBeginA(), n.getEndA(),
c.getBeginB(), n.getEndB()));
wordEdits.remove(j + 1);
continue;
}
j++;
}
edits.set(i, new ReplaceEdit(e, wordEdits));
}
}
return new PatchListEntry(fileHeader, edits);
}
private static Text read(Repository repo, String path, RevTree tree)
throws IOException {
TreeWalk tw = TreeWalk.forPath(repo, path, tree);
if (tw == null || tw.getFileMode(0).getObjectType() != Constants.OBJ_BLOB) {
return Text.EMPTY;
}
ObjectLoader ldr = repo.openObject(tw.getObjectId(0));
if (ldr == null) {
return Text.EMPTY;
}
return new Text(ldr.getCachedBytes());
}
private static AnyObjectId aFor(final PatchListKey key,
final Repository repo, final RevCommit b) throws IOException {
if (key.getOldId() != null) {

56
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListEntry.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.Patch.ChangeType;
import com.google.gerrit.reviewdb.Patch.PatchType;
import org.eclipse.jgit.diff.Edit;
import org.eclipse.jgit.diff.ReplaceEdit;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.patch.CombinedFileHeader;
@@ -59,7 +60,7 @@ public class PatchListEntry {
private final byte[] header;
private final List<Edit> edits;
PatchListEntry(final FileHeader hdr) {
PatchListEntry(final FileHeader hdr, List<Edit> editList) {
changeType = toChangeType(hdr);
patchType = toPatchType(hdr);
@@ -93,7 +94,7 @@ public class PatchListEntry {
|| hdr.getNewMode() == FileMode.GITLINK) {
edits = Collections.emptyList();
} else {
edits = Collections.unmodifiableList(hdr.toEditList());
edits = Collections.unmodifiableList(editList);
}
}
@@ -156,13 +157,27 @@ public class PatchListEntry {
writeVarInt32(out, edits.size());
for (final Edit e : edits) {
writeVarInt32(out, e.getBeginA());
writeVarInt32(out, e.getEndA());
writeVarInt32(out, e.getBeginB());
writeVarInt32(out, e.getEndB());
write(out, e);
if (e instanceof ReplaceEdit) {
ReplaceEdit r = (ReplaceEdit) e;
writeVarInt32(out, r.getInternalEdits().size());
for (Edit i : r.getInternalEdits()) {
write(out, i);
}
} else {
writeVarInt32(out, 0);
}
}
}
private void write(final OutputStream out, final Edit e) throws IOException {
writeVarInt32(out, e.getBeginA());
writeVarInt32(out, e.getEndA());
writeVarInt32(out, e.getBeginB());
writeVarInt32(out, e.getEndB());
}
static PatchListEntry readFrom(final InputStream in) throws IOException {
final ChangeType changeType = readEnum(in, ChangeType.values());
final PatchType patchType = readEnum(in, PatchType.values());
@@ -173,15 +188,32 @@ public class PatchListEntry {
final int editCount = readVarInt32(in);
final Edit[] editArray = new Edit[editCount];
for (int i = 0; i < editCount; i++) {
final int beginA = readVarInt32(in);
final int endA = readVarInt32(in);
final int beginB = readVarInt32(in);
final int endB = readVarInt32(in);
editArray[i] = new Edit(beginA, endA, beginB, endB);
editArray[i] = readEdit(in);
int innerCount = readVarInt32(in);
if (0 < innerCount) {
Edit[] inner = new Edit[innerCount];
for (int innerIdx = 0; innerIdx < innerCount; innerIdx++) {
inner[innerIdx] = readEdit(in);
}
editArray[i] = new ReplaceEdit(editArray[i], toList(inner));
}
}
return new PatchListEntry(changeType, patchType, oldName, newName, hdr,
Collections.unmodifiableList(Arrays.asList(editArray)));
toList(editArray));
}
private static List<Edit> toList(Edit[] l) {
return Collections.unmodifiableList(Arrays.asList(l));
}
private static Edit readEdit(final InputStream in) throws IOException {
final int beginA = readVarInt32(in);
final int endA = readVarInt32(in);
final int beginB = readVarInt32(in);
final int endB = readVarInt32(in);
return new Edit(beginA, endA, beginB, endB);
}
private static byte[] compact(final FileHeader h) {

2
gerrit-server/src/main/java/com/google/gerrit/server/patch/PatchListKey.java
View File

@@ -35,7 +35,7 @@ import java.io.Serializable;
import javax.annotation.Nullable;
public class PatchListKey implements Serializable {
static final long serialVersionUID = 9L;
static final long serialVersionUID = 10L;
private transient ObjectId oldId;
private transient ObjectId newId;

43
gerrit-server/src/main/java/com/google/gerrit/server/patch/Text.java
View File

@@ -15,18 +15,20 @@
package com.google.gerrit.server.patch;
import org.eclipse.jgit.diff.RawText;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.util.RawParseUtils;
import org.mozilla.universalchardet.UniversalDetector;
import java.io.UnsupportedEncodingException;
import java.nio.charset.Charset;
public class Text extends RawText {
public static final byte[] NO_BYTES = {};
public static final Text EMPTY = new Text(NO_BYTES);
public static String asString(byte[] content, String encoding)
throws UnsupportedEncodingException {
public static String asString(byte[] content, String encoding) {
return new String(content, charset(content, encoding));
}
private static Charset charset(byte[] content, String encoding) {
if (encoding == null) {
UniversalDetector d = new UniversalDetector(null);
d.handleData(content, 0, content.length);
@@ -36,9 +38,11 @@ public class Text extends RawText {
if (encoding == null) {
encoding = "ISO-8859-1";
}
return new String(content, encoding);
return Charset.forName(encoding);
}
private Charset charset;
public Text(final byte[] r) {
super(r);
}
@@ -48,11 +52,34 @@ public class Text extends RawText {
}
public String getLine(final int i) {
final int s = lines.get(i + 1);
int e = lines.get(i + 2);
return getLines(i, i + 1);
}
public String getLines(final int begin, final int end) {
if (begin == end) {
return "";
}
final int s = getLineStart(begin);
int e = getLineEnd(end - 1);
if (content[e - 1] == '\n') {
e--;
}
return RawParseUtils.decode(Constants.CHARSET, content, s, e);
return decode(s, e);
}
private String decode(final int s, int e) {
if (charset == null) {
charset = charset(content, null);
}
return RawParseUtils.decode(charset, content, s, e);
}
private int getLineStart(final int i) {
return lines.get(i + 1);
}
private int getLineEnd(final int i) {
return lines.get(i + 2);
}
}