Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Wolf2015-09-12 12:59:47 +0000
committerMatthias Sohn2015-09-16 23:19:45 +0000
commit0da61c5dd63a3fb79f293b785d5baae01c0c5be7 (patch)
treec7aa9d2626e4442f3c1763711a2eb88bf556702d
parent039ed922905bb27e496282b931cdd7b5e90b2e57 (diff)
downloadegit-0da61c5dd63a3fb79f293b785d5baae01c0c5be7.tar.gz
egit-0da61c5dd63a3fb79f293b785d5baae01c0c5be7.tar.xz
egit-0da61c5dd63a3fb79f293b785d5baae01c0c5be7.zip
Improve performance of HyperlinkTokenScanner
My initial implementation of this was too inefficient. It invoked the HyperlinkDetectors for every character, which effectively re-introduced bug 415537 and made editing long commit messages with only a few links a pain. Therefore, compute all hyperlinks on a line and cache them once computed. Use this cache while we're on that line, and re-compute it when the current position passes to a new line. Add an internal subclass of the standard URL hyperlink detector org.eclipse.jface.text.hyperlink.URLHyperlinkDetector because that one is only capable of finding hyperlinks at the start of the region. Others, like those from mylyn, do find all links in a region on their own. For an example commit message see: https://bugs.eclipse.org/bugs/show_bug.cgi?id=415537 Change-Id: I464f321d5f5d62836d031615b0c6252a54ee893f Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch> Signed-off-by: Matthias Sohn <matthias.sohn@sap.com>
-rw-r--r--org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkTokenScanner.java166
1 files changed, 154 insertions, 12 deletions
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkTokenScanner.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkTokenScanner.java
index 91517a5a6f..e18d8c632a 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkTokenScanner.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/dialogs/HyperlinkTokenScanner.java
@@ -8,16 +8,25 @@
*******************************************************************************/
package org.eclipse.egit.ui.internal.dialogs;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.List;
+
import org.eclipse.core.runtime.Assert;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jface.resource.JFaceColors;
+import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.IDocument;
import org.eclipse.jface.text.IRegion;
+import org.eclipse.jface.text.ITextViewer;
import org.eclipse.jface.text.Region;
import org.eclipse.jface.text.TextAttribute;
import org.eclipse.jface.text.hyperlink.IHyperlink;
import org.eclipse.jface.text.hyperlink.IHyperlinkDetector;
+import org.eclipse.jface.text.hyperlink.URLHyperlinkDetector;
import org.eclipse.jface.text.rules.IToken;
import org.eclipse.jface.text.rules.ITokenScanner;
import org.eclipse.jface.text.rules.Token;
@@ -32,12 +41,20 @@ public class HyperlinkTokenScanner implements ITokenScanner {
private int tokenStart;
+ private int lastLineStart;
+
private IToken hyperlinkToken;
private final ISourceViewer viewer;
private final IHyperlinkDetector[] hyperlinkDetectors;
+ /**
+ * Caches all hyperlinks on a line to avoid calling the hyperlink detectors
+ * too often.
+ */
+ private final List<IHyperlink> hyperlinksOnLine = new ArrayList<>();
+
/** The current offset in the document. */
protected int currentOffset;
@@ -76,7 +93,20 @@ public class HyperlinkTokenScanner implements ITokenScanner {
*/
public HyperlinkTokenScanner(IHyperlinkDetector[] hyperlinkDetectors,
ISourceViewer viewer, @Nullable TextAttribute defaultAttribute) {
- this.hyperlinkDetectors = hyperlinkDetectors;
+ IHyperlinkDetector[] allDetectors;
+ if (hyperlinkDetectors == null || hyperlinkDetectors.length == 0) {
+ allDetectors = new IHyperlinkDetector[0];
+ } else {
+ allDetectors = new IHyperlinkDetector[hyperlinkDetectors.length
+ + 1];
+ System.arraycopy(hyperlinkDetectors, 0, allDetectors, 0,
+ hyperlinkDetectors.length);
+ // URLHyperlinkDetector can only detect hyperlinks at the start of
+ // the range. We need one that can detect all hyperlinks in a given
+ // region.
+ allDetectors[hyperlinkDetectors.length] = new MultiURLHyperlinkDetector();
+ }
+ this.hyperlinkDetectors = allDetectors;
this.viewer = viewer;
this.defaultToken = new Token(defaultAttribute);
}
@@ -89,21 +119,72 @@ public class HyperlinkTokenScanner implements ITokenScanner {
@Override
public IToken nextToken() {
- this.tokenStart = currentOffset;
+ tokenStart = currentOffset;
if (currentOffset >= endOfRange) {
+ hyperlinksOnLine.clear();
return Token.EOF;
}
- for (IHyperlinkDetector hyperlinkDetector : hyperlinkDetectors) {
- IHyperlink[] newLinks = hyperlinkDetector.detectHyperlinks(viewer,
- new Region(currentOffset, 0), false);
- if (newLinks != null && newLinks.length > 0) {
- IRegion region = newLinks[0].getHyperlinkRegion();
- int end = Math.min(endOfRange,
- region.getOffset() + region.getLength());
- if (end > tokenStart) {
- currentOffset = end;
- return hyperlinkToken;
+ if (hyperlinkDetectors.length > 0) {
+ try {
+ IRegion currentLine = document
+ .getLineInformationOfOffset(currentOffset);
+ if (currentLine.getOffset() != lastLineStart) {
+ // Compute all hyperlinks in the line
+ hyperlinksOnLine.clear();
+ for (IHyperlinkDetector hyperlinkDetector : hyperlinkDetectors) {
+ IHyperlink[] newLinks = hyperlinkDetector
+ .detectHyperlinks(viewer, currentLine, false);
+ if (newLinks != null && newLinks.length > 0) {
+ Collections.addAll(hyperlinksOnLine, newLinks);
+ }
+ }
+ // Sort them by offset, and with increasing length
+ Collections.sort(hyperlinksOnLine,
+ new Comparator<IHyperlink>() {
+ @Override
+ public int compare(IHyperlink a, IHyperlink b) {
+ int diff = a.getHyperlinkRegion()
+ .getOffset()
+ - b.getHyperlinkRegion()
+ .getOffset();
+ if (diff != 0) {
+ return diff;
+ }
+ return a.getHyperlinkRegion().getLength()
+ - b.getHyperlinkRegion()
+ .getLength();
+ }
+ });
+ lastLineStart = currentLine.getOffset();
+ }
+ if (!hyperlinksOnLine.isEmpty()) {
+ // Find first hyperlink for the position. We may have to
+ // skip a few in case there are several hyperlinks at the
+ // same position and with the same length.
+ Iterator<IHyperlink> iterator = hyperlinksOnLine.iterator();
+ while (iterator.hasNext()) {
+ IHyperlink next = iterator.next();
+ IRegion linkRegion = next.getHyperlinkRegion();
+ int linkEnd = linkRegion.getOffset()
+ + linkRegion.getLength();
+ if (currentOffset >= linkEnd) {
+ iterator.remove();
+ } else if (linkRegion.getOffset() <= currentOffset) {
+ // This is our link
+ iterator.remove();
+ int end = Math.min(endOfRange, linkEnd);
+ if (end > currentOffset) {
+ currentOffset = end;
+ return hyperlinkToken;
+ }
+ } else {
+ // Next hyperlink is beyond current position
+ break;
+ }
+ }
}
+ } catch (BadLocationException e) {
+ // Ignore and keep going
}
}
int actualOffset = currentOffset;
@@ -144,6 +225,7 @@ public class HyperlinkTokenScanner implements ITokenScanner {
int length, @Nullable Color color) {
Assert.isTrue(document == viewer.getDocument());
this.document = document;
+ this.lastLineStart = -1;
this.endOfRange = offset + length;
this.currentOffset = offset;
this.tokenStart = -1;
@@ -161,4 +243,64 @@ public class HyperlinkTokenScanner implements ITokenScanner {
protected IToken scanToken() {
return null;
}
+
+ /**
+ * A {@link URLHyperlinkDetector} that returns all hyperlinks in a region.
+ * <p>
+ * This internal class assumes that the region is either empty or else spans
+ * a whole line.
+ * </p>
+ */
+ private static class MultiURLHyperlinkDetector
+ extends URLHyperlinkDetector {
+
+ @Override
+ public IHyperlink[] detectHyperlinks(ITextViewer textViewer,
+ IRegion region, boolean canShowMultipleHyperlinks) {
+ if (region.getLength() == 0) {
+ return super.detectHyperlinks(textViewer, region,
+ canShowMultipleHyperlinks);
+ }
+ // URLHyperlinkDetector only finds hyperlinks at the region start.
+ // We know here that the given region spans a whole line since we're
+ // only called from HyperlinkTokenScanner.nextToken().
+ try {
+ String line = textViewer.getDocument().get(region.getOffset(),
+ region.getLength());
+ int currentOffset = region.getOffset();
+ int lineStart = currentOffset;
+ int regionEnd = currentOffset + region.getLength();
+ List<IHyperlink> allLinks = new ArrayList<>();
+ while (currentOffset < regionEnd) {
+ IHyperlink[] newLinks = super.detectHyperlinks(
+ textViewer, new Region(currentOffset, 0),
+ canShowMultipleHyperlinks);
+ currentOffset++;
+ if (newLinks != null && newLinks.length > 0) {
+ Collections.addAll(allLinks, newLinks);
+ for (IHyperlink link : newLinks) {
+ int end = link.getHyperlinkRegion().getOffset()
+ + link.getHyperlinkRegion().getLength();
+ if (end > currentOffset) {
+ currentOffset = end;
+ }
+ }
+ }
+ // Advance to the next "://" combination.
+ int nextCandidatePos = lineStart
+ + line.indexOf("://", currentOffset - lineStart); //$NON-NLS-1$
+ if (nextCandidatePos > currentOffset) {
+ currentOffset = nextCandidatePos;
+ } else if (nextCandidatePos < currentOffset) {
+ // No more links.
+ break;
+ }
+ }
+ return allLinks.toArray(new IHyperlink[allLinks.size()]);
+ } catch (BadLocationException e) {
+ return null;
+ }
+ }
+
+ }
} \ No newline at end of file

Back to the top