From 7dd99fe7474604f314c01bcd4123eb7cbacfb583 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 11 Nov 2014 07:52:15 +0100 Subject: Image diffs Ticket 88: https://dev.gitblit.com/tickets/gitblit.git/88 Based on Lea Verou's pure CSS slider: http://lea.verou.me/2014/07/image-comparison-slider-with-pure-css/ * Add a callback interface, pass it through DiffUtils to the GitBlitDiffFormatter. Is needed because the rendering needs access to the repositoryName and other things that are known only at higher levels. * New class ImageDiffHandler responsible for rendering an image diff. Called for all binary diffs, doesn't do anything if it's not an image. HTML is generated via JSoup: no worries about forgetting to close a tag, not about HTML escaping, nor about XSS. * The 3 diff pages set up such an ImageDIffHandler and pass it along. * CSS changes: from Lea Verou, with some minor improvements. I think in the long run there'll be no way around rewriting the HTML diff formatter from scratch, not using the standard JGit DiffFormatter at all. --- .../com/gitblit/utils/GitBlitDiffFormatter.java | 82 +++++++++++++++++----- 1 file changed, 63 insertions(+), 19 deletions(-) (limited to 'src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java') diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index 5de9e50a..edaed70f 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -19,8 +19,10 @@ import static org.eclipse.jgit.lib.Constants.encode; import static org.eclipse.jgit.lib.Constants.encodeASCII; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.text.MessageFormat; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -34,6 +36,7 @@ import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.util.RawParseUtils; import com.gitblit.models.PathModel.PathChangeModel; +import com.gitblit.utils.DiffUtils.BinaryDiffHandler; import com.gitblit.utils.DiffUtils.DiffStat; import com.gitblit.wicket.GitBlitWebApp; @@ -71,7 +74,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { */ private static final int GLOBAL_DIFF_LIMIT = 20000; - private final ResettableByteArrayOutputStream os; + private final DiffOutputStream os; private final DiffStat diffStat; @@ -122,9 +125,45 @@ public class GitBlitDiffFormatter extends DiffFormatter { /** If {@link #truncated}, contains all entries skipped. */ private final List skipped = new ArrayList(); - public GitBlitDiffFormatter(String commitId, String path) { - super(new ResettableByteArrayOutputStream()); - this.os = (ResettableByteArrayOutputStream) getOutputStream(); + /** + * A {@link ResettableByteArrayOutputStream} that intercept the "Binary files differ" message produced + * by the super implementation. Unfortunately the super implementation has far too many things private; + * otherwise we'd just have re-implemented {@link GitBlitDiffFormatter#format(DiffEntry) format(DiffEntry)} + * completely without ever calling the super implementation. + */ + private static class DiffOutputStream extends ResettableByteArrayOutputStream { + + private static final String BINARY_DIFFERENCE = "Binary files differ\n"; + + private GitBlitDiffFormatter formatter; + private BinaryDiffHandler binaryDiffHandler; + + public void setFormatter(GitBlitDiffFormatter formatter, BinaryDiffHandler handler) { + this.formatter = formatter; + this.binaryDiffHandler = handler; + } + + @Override + public void write(byte[] b, int offset, int length) { + if (binaryDiffHandler != null + && RawParseUtils.decode(Arrays.copyOfRange(b, offset, offset + length)).contains(BINARY_DIFFERENCE)) + { + String binaryDiff = binaryDiffHandler.renderBinaryDiff(formatter.entry); + if (binaryDiff != null) { + byte[] bb = ("" + binaryDiff + "").getBytes(StandardCharsets.UTF_8); + super.write(bb, 0, bb.length); + return; + } + } + super.write(b, offset, length); + } + + } + + public GitBlitDiffFormatter(String commitId, String path, BinaryDiffHandler handler) { + super(new DiffOutputStream()); + this.os = (DiffOutputStream) getOutputStream(); + this.os.setFormatter(this, handler); this.diffStat = new DiffStat(commitId); // If we have a full commitdiff, install maxima to avoid generating a super-long diff listing that // will only tax the browser too much. @@ -217,7 +256,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { super.format(ent); if (!truncated) { // Close the table - os.write("
\n".getBytes()); + os.write("\n".getBytes()); } } @@ -235,13 +274,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { private void reset() { if (!isOff) { os.resetTo(startCurrent); - try { - os.write("".getBytes()); - os.write(StringUtils.escapeForHtml(getMsg("gb.diffFileDiffTooLarge", "Diff too large"), false).getBytes()); - os.write("\n".getBytes()); - } catch (IOException ex) { - // Cannot happen with a ByteArrayOutputStream - } + writeFullWidthLine(getMsg("gb.diffFileDiffTooLarge", "Diff too large")); totalNofLinesCurrent = totalNofLinesPrevious; isOff = true; } @@ -277,13 +310,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { default: return; } - try { - os.write("".getBytes()); - os.write(StringUtils.escapeForHtml(message, false).getBytes()); - os.write("\n".getBytes()); - } catch (IOException ex) { - // Cannot happen with a ByteArrayOutputStream - } + writeFullWidthLine(message); } /** @@ -355,6 +382,22 @@ public class GitBlitDiffFormatter extends DiffFormatter { } } + /** + * Writes a line spanning the full width of the code view, including the gutter. + * + * @param text + * to put on that line; will be HTML-escaped. + */ + private void writeFullWidthLine(String text) { + try { + os.write("".getBytes()); + os.write(StringUtils.escapeForHtml(text, false).getBytes()); + os.write("\n".getBytes()); + } catch (IOException ex) { + // Cannot happen with a ByteArrayOutputStream + } + } + @Override protected void writeLine(final char prefix, final RawText text, final int cur) throws IOException { if (nofLinesCurrent++ == 0) { @@ -453,6 +496,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { if (gitLinkDiff) { sb.append(""); } + sb.append('\n'); } } if (truncated) { -- cgit v1.2.3 From 3e1336cc6d32511daf2acab9c45a517cd3b10058 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 12 Nov 2014 20:31:12 +0100 Subject: Opacity adjustments for image diffs * ImageDiffHandler adds the slider; styled in gitblit.css * imgdiff.js is a little bottom-loaded Javascript that adjusts the opacity on sliders' scroll events. * The three diff pages add this bottom script to the page if needed * GitBlitDiffFormatter: center image diffs. --- .../com/gitblit/utils/GitBlitDiffFormatter.java | 2 +- .../com/gitblit/wicket/pages/BlobDiffPage.java | 6 +++++ .../com/gitblit/wicket/pages/CommitDiffPage.java | 3 +++ .../java/com/gitblit/wicket/pages/ComparePage.java | 3 +++ .../com/gitblit/wicket/pages/ImageDiffHandler.java | 16 +++++++++-- .../com/gitblit/wicket/pages/scripts/imgdiff.js | 23 ++++++++++++++++ src/main/resources/gitblit.css | 31 +++++++++++++++++++++- 7 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js (limited to 'src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java') diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index edaed70f..3c65267b 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -150,7 +150,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { { String binaryDiff = binaryDiffHandler.renderBinaryDiff(formatter.entry); if (binaryDiff != null) { - byte[] bb = ("" + binaryDiff + "").getBytes(StandardCharsets.UTF_8); + byte[] bb = ("" + binaryDiff + "").getBytes(StandardCharsets.UTF_8); super.write(bb, 0, bb.length); return; } diff --git a/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java b/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java index 517e80b8..71516ec8 100644 --- a/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java +++ b/src/main/java/com/gitblit/wicket/pages/BlobDiffPage.java @@ -55,6 +55,9 @@ public class BlobDiffPage extends RepositoryPage { ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName, parent.getName(), commit.getName(), imageExtensions); diff = DiffUtils.getDiff(r, commit, blobPath, DiffOutputType.HTML, handler).content; + if (handler.getImgDiffCount() > 0) { + addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs + } add(new BookmarkablePageLink("patchLink", PatchPage.class, WicketUtils.newPathParameter(repositoryName, objectId, blobPath))); } else { @@ -63,6 +66,9 @@ public class BlobDiffPage extends RepositoryPage { ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName, baseCommit.getName(), commit.getName(), imageExtensions); diff = DiffUtils.getDiff(r, baseCommit, commit, blobPath, DiffOutputType.HTML, handler).content; + if (handler.getImgDiffCount() > 0) { + addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs + } add(new BookmarkablePageLink("patchLink", PatchPage.class, WicketUtils.newBlobDiffParameter(repositoryName, baseObjectId, objectId, blobPath))); diff --git a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java index 77d5ccf3..e40af515 100644 --- a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java +++ b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java @@ -85,6 +85,9 @@ public class CommitDiffPage extends RepositoryPage { final ImageDiffHandler handler = new ImageDiffHandler(getContextUrl(), repositoryName, parents.isEmpty() ? null : parents.get(0), commit.getName(), imageExtensions); final DiffOutput diff = DiffUtils.getCommitDiff(r, commit, DiffOutputType.HTML, handler); + if (handler.getImgDiffCount() > 0) { + addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs + } // add commit diffstat int insertions = 0; diff --git a/src/main/java/com/gitblit/wicket/pages/ComparePage.java b/src/main/java/com/gitblit/wicket/pages/ComparePage.java index dae8d8ea..c0141eba 100644 --- a/src/main/java/com/gitblit/wicket/pages/ComparePage.java +++ b/src/main/java/com/gitblit/wicket/pages/ComparePage.java @@ -117,6 +117,9 @@ public class ComparePage extends RepositoryPage { fromCommit.getName(), toCommit.getName(), imageExtensions); final DiffOutput diff = DiffUtils.getDiff(r, fromCommit, toCommit, DiffOutputType.HTML, handler); + if (handler.getImgDiffCount() > 0) { + addBottomScript("scripts/imgdiff.js"); // Tiny support script for image diffs + } // add compare diffstat int insertions = 0; diff --git a/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java b/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java index 69d84f4e..4278f238 100644 --- a/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java +++ b/src/main/java/com/gitblit/wicket/pages/ImageDiffHandler.java @@ -40,6 +40,8 @@ public class ImageDiffHandler implements DiffUtils.BinaryDiffHandler { private final String baseUrl; private final List imageExtensions; + private int imgDiffCount = 0; + public ImageDiffHandler(final String baseUrl, final String repositoryName, final String oldCommitId, final String newCommitId, final List imageExtensions) { this.baseUrl = baseUrl; @@ -62,8 +64,10 @@ public class ImageDiffHandler implements DiffUtils.BinaryDiffHandler { String oldUrl = getImageUrl(diffEntry, Side.OLD); String newUrl = getImageUrl(diffEntry, Side.NEW); if (oldUrl != null && newUrl != null) { + imgDiffCount++; + String id = "imgdiff" + imgDiffCount; HtmlBuilder builder = new HtmlBuilder("div"); - Element container = builder.root().appendElement("div").attr("class", "imgdiff"); + Element container = builder.root().attr("align", "center").appendElement("div").attr("class", "imgdiff"); Element resizeable = container.appendElement("div").attr("class", "imgdiff-left"); // style='max-width:640px;' is necessary for ensuring that the browser limits large images // to some reasonable width, and to override the "img { max-width: 100%; }" from bootstrap.css, @@ -73,8 +77,11 @@ public class ImageDiffHandler implements DiffUtils.BinaryDiffHandler { // is too wide. // XXX: Maybe add a max-height, too, to limit portrait-oriented images to some reasonable height? // (Like a 300x10000px image...) - resizeable.appendElement("img").attr("class", "imgdiff imgdiff-left").attr("style", "max-width:640px;").attr("src", oldUrl); + resizeable.appendElement("img").attr("class", "imgdiff-left").attr("id", id).attr("style", "max-width:640px;").attr("src", oldUrl); container.appendElement("img").attr("class", "imgdiff").attr("style", "max-width:640px;").attr("src", newUrl); + builder.root().appendElement("br"); + Element slider = builder.root().appendElement("div").attr("class", "imgdiff-slider").attr("id", "slider-" + id); + slider.appendElement("div").attr("class", "imgdiff-slider-inner"); return builder.toString(); } break; @@ -90,6 +97,11 @@ public class ImageDiffHandler implements DiffUtils.BinaryDiffHandler { return null; } + /** Returns the number of image diffs generated so far by this {@link ImageDiffHandler}. */ + public int getImgDiffCount() { + return imgDiffCount; + } + /** * Constructs a URL that will fetch the designated resource in the git repository. The returned string will * contain the URL fully URL-escaped, but note that it may still contain unescaped ampersands, so the result diff --git a/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js b/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js new file mode 100644 index 00000000..faa2f334 --- /dev/null +++ b/src/main/java/com/gitblit/wicket/pages/scripts/imgdiff.js @@ -0,0 +1,23 @@ +/* + * Copyright 2014 Tom + * + * 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. + */ +jQuery(function () { + // Runs on jQuery's document.ready and sets up the scroll event handlers for all image diffs. + jQuery(".imgdiff-slider").scroll(function() { + var w = 1.0 - (this.scrollLeft / (this.scrollWidth - (this.clientWidth || this.offsetWidth))); + // We encode the target img id in the slider's id: slider-imgdiffNNN. + jQuery('#' + this.id.substr(this.id.indexOf('-') + 1)).css("opacity", w); + }) +}); diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index f6d6b24d..bdab2056 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -1486,14 +1486,43 @@ div.imgdiff-left:before { img.imgdiff-left { margin-left: 18px; /* Compensate for padding on outer div. */ + user-select: none; } img.imagediff { user-select: none; + /* Checkerboard background */ + background-color: white; + background-image: linear-gradient(45deg, #DDD 25%, transparent 25%, transparent 75%, #DDD 75%, #DDD), linear-gradient(45deg, #DDD 25%, transparent 25%, transparent 75%, #DDD 75%, #DDD); + background-size: 16px 16px; + background-position: 0 0, 8px 8px; } .diff-img { - margin: 2px 2px; + margin: 2px; +} + +div.imgdiff-slider { + display: inline-block; + position: relative; + margin: 0px 2px; + width: 420px; + max-width: 420px; + height: 24px; + min-height: 18px; + overflow-x: scroll; + background: linear-gradient(to right, #F00, #0F0); +} + +div.imgdiff-slider-inner { + position: absolute; + bottom: 0; + margin: 0; + padding: 0; + width : 1000%; + height: 1px; + border: none; + background: transparent; } /* End image diffs */ -- cgit v1.2.3