summaryrefslogtreecommitdiffstatsabout
diff options
context:
space:
mode:
authorSteffen Pingel2012-02-04 09:42:30 (EST)
committer Benjamin Muskalla2012-03-23 11:35:29 (EDT)
commit31a9c126a5ad439cba2bdf3ddb04bac725fda1c9 (patch)
treec2af906f13b371a162f04730710e2a51589c94ca
parente0c4133d00ac9ef7b2056ff91ac2bdfd076336c7 (diff)
downloadegit-31a9c126a5ad439cba2bdf3ddb04bac725fda1c9.zip
egit-31a9c126a5ad439cba2bdf3ddb04bac725fda1c9.tar.gz
egit-31a9c126a5ad439cba2bdf3ddb04bac725fda1c9.tar.bz2
Fix regions decorated by commit hyperlink detectorrefs/changes/54/5054/2
* The offset of the hyperlinked region is now calculated correctly. * Gerrit change ids are not detected as commit ids. * Performance is improved through using a regular expression rather than tokenizing the text. Bug: 355868 Change-Id: I35e36dc057a4ce1279836cf1aa65cbea6f83dbdb
-rw-r--r--org.eclipse.egit.mylyn.ui.test/src/org/eclipse/egit/internal/mylyn/CommitHyperlinkDetectorTest.java24
-rw-r--r--org.eclipse.egit.mylyn.ui/src/org/eclipse/egit/internal/mylyn/ui/CommitHyperlinkDetector.java134
2 files changed, 106 insertions, 52 deletions
diff --git a/org.eclipse.egit.mylyn.ui.test/src/org/eclipse/egit/internal/mylyn/CommitHyperlinkDetectorTest.java b/org.eclipse.egit.mylyn.ui.test/src/org/eclipse/egit/internal/mylyn/CommitHyperlinkDetectorTest.java
index b8b97c1..1f16511 100644
--- a/org.eclipse.egit.mylyn.ui.test/src/org/eclipse/egit/internal/mylyn/CommitHyperlinkDetectorTest.java
+++ b/org.eclipse.egit.mylyn.ui.test/src/org/eclipse/egit/internal/mylyn/CommitHyperlinkDetectorTest.java
@@ -118,6 +118,30 @@ public class CommitHyperlinkDetectorTest {
assertNull(hyperlinks);
}
+ @Test
+ public void testMultiLine() {
+ setText("Test multi-line text\n" + EXAMPLE_ID);
+ IHyperlink[] hyperlinks = detectHyperlinks(0,textViewer.getDocument().getLength());
+ assertEquals(1, hyperlinks.length);
+ assertEquals(EXAMPLE_ID, hyperlinks[0].getHyperlinkText());
+ assertEquals(new Region(21, EXAMPLE_ID.length()), hyperlinks[0].getHyperlinkRegion());
+ }
+
+ @Test
+ public void testGerritId() {
+ setText("I" + EXAMPLE_ID);
+ IHyperlink[] hyperlinks = detectHyperlinks(0,textViewer.getDocument().getLength());
+ assertNull(hyperlinks);
+ }
+
+ @Test
+ public void testGerritIdWithinText() {
+ setText("abc I" + EXAMPLE_ID);
+ IHyperlink[] hyperlinks = detectHyperlinks(5,textViewer.getDocument().getLength());
+ assertNull(hyperlinks);
+ }
+
+
private IHyperlink[] detectHyperlinks() {
return detectHyperlinks(0, textViewer.getDocument().getLength());
}
diff --git a/org.eclipse.egit.mylyn.ui/src/org/eclipse/egit/internal/mylyn/ui/CommitHyperlinkDetector.java b/org.eclipse.egit.mylyn.ui/src/org/eclipse/egit/internal/mylyn/ui/CommitHyperlinkDetector.java
index cacd848..34fec85 100644
--- a/org.eclipse.egit.mylyn.ui/src/org/eclipse/egit/internal/mylyn/ui/CommitHyperlinkDetector.java
+++ b/org.eclipse.egit.mylyn.ui/src/org/eclipse/egit/internal/mylyn/ui/CommitHyperlinkDetector.java
@@ -15,6 +15,8 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.eclipse.core.runtime.Assert;
import org.eclipse.egit.core.Activator;
@@ -33,7 +35,6 @@ import org.eclipse.jface.text.hyperlink.AbstractHyperlinkDetector;
import org.eclipse.jface.text.hyperlink.IHyperlink;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
-import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
@@ -47,6 +48,9 @@ import org.eclipse.swt.widgets.Shell;
*/
public class CommitHyperlinkDetector extends AbstractHyperlinkDetector {
+ private static final Pattern PATTERN_COMMIT_ID = Pattern
+ .compile("(?<!\\w)([0-9a-f]{8}([0-9a-f]{32})?)"); //$NON-NLS-1$
+
private static class CommitHyperlink implements IHyperlink {
private IRegion region;
@@ -136,67 +140,69 @@ public class CommitHyperlinkDetector extends AbstractHyperlinkDetector {
}
+ @Override
+ public String toString() {
+ StringBuilder builder = new StringBuilder();
+ builder.append("CommitHyperlink [region="); //$NON-NLS-1$
+ builder.append(region);
+ builder.append(", objectId="); //$NON-NLS-1$
+ builder.append(objectId);
+ builder.append("]"); //$NON-NLS-1$
+ return builder.toString();
+ }
+
}
/**
- * Detects and returns all available hyperlinks for the given {@link TextViewer} which link to a Git commit.
+ * Detects and returns all available hyperlinks for the given
+ * {@link TextViewer} which link to a Git commit.
*/
public IHyperlink[] detectHyperlinks(ITextViewer textViewer,
- IRegion region, boolean canShowMultipleHyperlinks) {
-
+ final IRegion region, boolean canShowMultipleHyperlinks) {
IDocument document = textViewer.getDocument();
- if (document == null || document.getLength() == 0)
+ if (document == null || document.getLength() == 0) {
return null;
+ }
String content;
- int offset = region.getOffset();
- int length = region.getLength();
+ int contentOffset;
+ int index;
try {
- if (length == 0) {
- IRegion lineInformation = document
- .getLineInformationOfOffset(offset);
- offset = lineInformation.getOffset();
- length = lineInformation.getLength();
+ if (region.getLength() == 0) {
+ // expand the region to include the whole line
+ IRegion lineInfo = document.getLineInformationOfOffset(region
+ .getOffset());
+ int lineLength = lineInfo.getLength();
+ int lineOffset = lineInfo.getOffset();
+ int lineEnd = lineOffset + lineLength;
+ int regionEnd = region.getOffset() + region.getLength();
+ if (lineOffset < region.getOffset()) {
+ int regionLength = Math.max(regionEnd, lineEnd)
+ - lineOffset;
+ contentOffset = lineOffset;
+ content = document.get(lineOffset, regionLength);
+ index = region.getOffset() - lineOffset;
+ } else {
+ // the line starts after region, may never happen
+ int regionLength = Math.max(regionEnd, lineEnd)
+ - region.getOffset();
+ contentOffset = region.getOffset();
+ content = document.get(contentOffset, regionLength);
+ index = 0;
+ }
+ } else {
+ content = document.get(region.getOffset(), region.getLength());
+ contentOffset = region.getOffset();
+ index = -1;
}
- content = document.get(offset, length);
- } catch (BadLocationException e) {
+ } catch (BadLocationException ex) {
return null;
}
- List<IHyperlink> hyperlinks = new ArrayList<IHyperlink>();
- String[] words = content.split(" "); //$NON-NLS-1$
- Shell shell = textViewer.getTextWidget().getShell();
- for (String potentialId : words) {
- String foundId = null;
- int foundOffset = 0;
- if (ObjectId.isId(potentialId)) {
- foundId = potentialId;
- foundOffset = offset;
- } else if (potentialId.length() > Constants.OBJECT_ID_STRING_LENGTH) {
- // could be beginning or end of a sentence
- String potentialIdAtBeginning = potentialId.substring(0,
- Constants.OBJECT_ID_STRING_LENGTH);
- if (ObjectId.isId(potentialIdAtBeginning)) {
- foundId = potentialIdAtBeginning;
- foundOffset = offset;
- } else {
- String potentialIdAtEnd = potentialId.substring(potentialId
- .length() - Constants.OBJECT_ID_STRING_LENGTH);
- if (ObjectId.isId(potentialIdAtEnd)) {
- foundId = potentialIdAtEnd;
- foundOffset = potentialId.length()
- - Constants.OBJECT_ID_STRING_LENGTH;
- }
-
- }
- }
- if (foundId != null) {
- CommitHyperlink hyperlink = new CommitHyperlink(new Region(
- foundOffset, Constants.OBJECT_ID_STRING_LENGTH),
- foundId, shell);
- hyperlinks.add(hyperlink);
- }
- offset += potentialId.length() + 1;
+ List<IHyperlink> hyperlinks = detectHyperlinks(textViewer, content,
+ index, contentOffset);
+ if (hyperlinks == null) {
+ return null;
}
// filter hyperlinks that do not match original region
@@ -204,17 +210,41 @@ public class CommitHyperlinkDetector extends AbstractHyperlinkDetector {
for (Iterator<IHyperlink> it = hyperlinks.iterator(); it.hasNext();) {
IHyperlink hyperlink = it.next();
IRegion hyperlinkRegion = hyperlink.getHyperlinkRegion();
- if (!isInRegion(region, hyperlinkRegion))
+ if (!isInRegion(region, hyperlinkRegion)) {
it.remove();
+ }
}
}
-
- if (hyperlinks.isEmpty())
+ if (hyperlinks.isEmpty()) {
return null;
-
+ }
return hyperlinks.toArray(new IHyperlink[hyperlinks.size()]);
}
+ private List<IHyperlink> detectHyperlinks(ITextViewer textViewer,
+ String content, int index, int contentOffset) {
+ Shell shell = textViewer.getTextWidget().getShell();
+ List<IHyperlink> links = null;
+ Matcher matcher = PATTERN_COMMIT_ID.matcher(content);
+ while (matcher.find()) {
+ if (index != -1
+ && (index < matcher.start() || index > matcher.end())) {
+ continue;
+ }
+ if (links == null) {
+ links = new ArrayList<IHyperlink>();
+ }
+ int start = matcher.start(1);
+ Region region = new Region(contentOffset + start, matcher.end(1)
+ - start);
+
+ CommitHyperlink hyperlink = new CommitHyperlink(region,
+ matcher.group(1), shell);
+ links.add(hyperlink);
+ }
+ return links;
+ }
+
private boolean isInRegion(IRegion detectInRegion, IRegion hyperlinkRegion) {
return detectInRegion.getOffset() >= hyperlinkRegion.getOffset()
&& detectInRegion.getOffset() <= hyperlinkRegion.getOffset()