diff options
author | Thomas Wolf | 2015-09-12 12:59:47 +0000 |
---|---|---|
committer | Matthias Sohn | 2015-09-16 23:19:45 +0000 |
commit | 0da61c5dd63a3fb79f293b785d5baae01c0c5be7 (patch) | |
tree | c7aa9d2626e4442f3c1763711a2eb88bf556702d | |
parent | 039ed922905bb27e496282b931cdd7b5e90b2e57 (diff) | |
download | egit-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.java | 166 |
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 |