From df0ba7f7ff02ed02c0ba7714ae928a79d932baef Mon Sep 17 00:00:00 2001 From: Tom Date: Sun, 26 Oct 2014 18:10:13 +0100 Subject: Improve the commitdiff. * Optimize CSS: simplify selectors. That alone cuts rendering time in half! * Adapt HTML generation accordingly. * Change line number generation so that one can select only code lines. Also move the +/- out of the code column; it also gets in the way when selecting. * Omit long diffs altogether. * Omit diff lines for deleted files, they're not particularly interesting. * Introduce a global limit on the maximum number of diff lines to show. * Supply translations for the languages I speak for the new messages. https://code.google.com/p/gitblit/issues/detail?id=450 was about a diff with nearly 300k changed lines (with more then 3000 files deleted). But one doesn't have to have such a monster commit to run into problems. My FF 32 become unresponsive for the 30+ seconds it takes it to render a commitdiff with some 30000 changed lines. (90% of which are in two generated files; the whole commit has just 20 files.) GitHub has no problems showing a commitdiff for this commit, but omits the two large generated files, which makes sense. This change implements a similar thing. Files with too many diff lines get omitted from the output, only the header and a message that the diff is too large remains. Additionally, there's a global limit on the length of a commitdiff; if we exceed that, the whole diff is truncated and the files not shown are listed. The CSS change improves performance by not using descendant selectors for all these table cells. Instead, we assign them precise classes and just use that in the CSS. The line number generation thing using data attributes and a :before selector in the CSS, which enables text selections only in the code column, is not strictly XHTML 1.0. (Data attributes are a feature of HTML 5.) However, reasonably modern browsers also handle this correctly if the page claims to be XHTML 1.0. Besides, the commitdiff page isn't XHTML compliant anyway; I don't think a pre-element may contain divs or even tables. (Note that this technique could be used on other diff pages, too. For instance on the blame page.) --- src/main/java/com/gitblit/utils/DiffUtils.java | 7 +- .../com/gitblit/utils/GitBlitDiffFormatter.java | 709 ++++++++++++++------- .../utils/ResettableByteArrayOutputStream.java | 42 ++ .../com/gitblit/wicket/GitBlitWebApp.properties | 9 +- .../com/gitblit/wicket/GitBlitWebApp_de.properties | 7 + .../com/gitblit/wicket/GitBlitWebApp_fr.properties | 7 + src/main/resources/gitblit.css | 85 ++- 7 files changed, 583 insertions(+), 283 deletions(-) create mode 100644 src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java diff --git a/src/main/java/com/gitblit/utils/DiffUtils.java b/src/main/java/com/gitblit/utils/DiffUtils.java index dd2a7807..f597b946 100644 --- a/src/main/java/com/gitblit/utils/DiffUtils.java +++ b/src/main/java/com/gitblit/utils/DiffUtils.java @@ -228,15 +228,16 @@ public class DiffUtils { DiffStat stat = null; String diff = null; try { - final ByteArrayOutputStream os = new ByteArrayOutputStream(); + ByteArrayOutputStream os = null; RawTextComparator cmp = RawTextComparator.DEFAULT; DiffFormatter df; switch (outputType) { case HTML: - df = new GitBlitDiffFormatter(os, commit.getName()); + df = new GitBlitDiffFormatter(commit.getName(), path); break; case PLAIN: default: + os = new ByteArrayOutputStream(); df = new DiffFormatter(os); break; } @@ -271,6 +272,7 @@ public class DiffUtils { } else { df.format(diffEntries); } + df.flush(); if (df instanceof GitBlitDiffFormatter) { // workaround for complex private methods in DiffFormatter diff = ((GitBlitDiffFormatter) df).getHtml(); @@ -278,7 +280,6 @@ public class DiffUtils { } else { diff = os.toString(); } - df.flush(); } catch (Throwable t) { LOGGER.error("failed to generate commit diff!", t); } diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index 47ff143a..0b16c374 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -1,236 +1,473 @@ -/* - * Copyright 2011 gitblit.com. - * - * 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.gitblit.utils; - -import static org.eclipse.jgit.lib.Constants.encode; -import static org.eclipse.jgit.lib.Constants.encodeASCII; - -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.OutputStream; -import java.text.MessageFormat; - -import org.eclipse.jgit.diff.DiffEntry; -import org.eclipse.jgit.diff.DiffFormatter; -import org.eclipse.jgit.diff.RawText; -import org.eclipse.jgit.util.RawParseUtils; - -import com.gitblit.models.PathModel.PathChangeModel; -import com.gitblit.utils.DiffUtils.DiffStat; - -/** - * Generates an html snippet of a diff in Gitblit's style, tracks changed paths, - * and calculates diff stats. - * - * @author James Moger - * - */ -public class GitBlitDiffFormatter extends DiffFormatter { - - private final OutputStream os; - - private final DiffStat diffStat; - - private PathChangeModel currentPath; - - private int left, right; - - public GitBlitDiffFormatter(OutputStream os, String commitId) { - super(os); - this.os = os; - this.diffStat = new DiffStat(commitId); - } - - @Override - public void format(DiffEntry ent) throws IOException { - currentPath = diffStat.addPath(ent); - super.format(ent); - } - - /** - * Output a hunk header - * - * @param aStartLine - * within first source - * @param aEndLine - * within first source - * @param bStartLine - * within second source - * @param bEndLine - * within second source - * @throws IOException - */ - @Override - protected void writeHunkHeader(int aStartLine, int aEndLine, int bStartLine, int bEndLine) - throws IOException { - os.write("....".getBytes()); - os.write('@'); - os.write('@'); - writeRange('-', aStartLine + 1, aEndLine - aStartLine); - writeRange('+', bStartLine + 1, bEndLine - bStartLine); - os.write(' '); - os.write('@'); - os.write('@'); - os.write("\n".getBytes()); - left = aStartLine + 1; - right = bStartLine + 1; - } - - protected void writeRange(final char prefix, final int begin, final int cnt) throws IOException { - os.write(' '); - os.write(prefix); - switch (cnt) { - case 0: - // If the range is empty, its beginning number must - // be the - // line just before the range, or 0 if the range is - // at the - // start of the file stream. Here, begin is always 1 - // based, - // so an empty file would produce "0,0". - // - os.write(encodeASCII(begin - 1)); - os.write(','); - os.write('0'); - break; - - case 1: - // If the range is exactly one line, produce only - // the number. - // - os.write(encodeASCII(begin)); - break; - - default: - os.write(encodeASCII(begin)); - os.write(','); - os.write(encodeASCII(cnt)); - break; - } - } - - @Override - protected void writeLine(final char prefix, final RawText text, final int cur) - throws IOException { - // update entry diffstat - currentPath.update(prefix); - - // output diff - os.write("".getBytes()); - switch (prefix) { - case '+': - os.write(("" + (right++) + "").getBytes()); - os.write("
".getBytes()); - break; - case '-': - os.write(("" + (left++) + "").getBytes()); - os.write("
".getBytes()); - break; - default: - os.write(("" + (left++) + "" + (right++) + "").getBytes()); - os.write("".getBytes()); - break; - } - os.write(prefix); - String line = text.getString(cur); - line = StringUtils.escapeForHtml(line, false); - os.write(encode(line)); - switch (prefix) { - case '+': - case '-': - os.write("
".getBytes()); - break; - default: - os.write("".getBytes()); - } - os.write("\n".getBytes()); - } - - /** - * Workaround function for complex private methods in DiffFormatter. This - * sets the html for the diff headers. - * - * @return - */ - public String getHtml() { - ByteArrayOutputStream bos = (ByteArrayOutputStream) os; - String html = RawParseUtils.decode(bos.toByteArray()); - String[] lines = html.split("\n"); - StringBuilder sb = new StringBuilder(); - boolean inFile = false; - String oldnull = "a/dev/null"; - for (String line : lines) { - if (line.startsWith("index")) { - // skip index lines - } else if (line.startsWith("new file")) { - // skip new file lines - } else if (line.startsWith("\\ No newline")) { - // skip no new line - } else if (line.startsWith("---") || line.startsWith("+++")) { - // skip --- +++ lines - } else if (line.startsWith("diff")) { - line = StringUtils.convertOctal(line); - if (line.indexOf(oldnull) > -1) { - // a is null, use b - line = line.substring(("diff --git " + oldnull).length()).trim(); - // trim b/ - line = line.substring(2).trim(); - } else { - // use a - line = line.substring("diff --git ".length()).trim(); - line = line.substring(line.startsWith("\"a/") ? 3 : 2); - line = line.substring(0, line.indexOf(" b/") > -1 ? line.indexOf(" b/") : line.indexOf("\"b/")).trim(); - } - - if (line.charAt(0) == '"') { - line = line.substring(1); - } - if (line.charAt(line.length() - 1) == '"') { - line = line.substring(0, line.length() - 1); - } - if (inFile) { - sb.append("
\n"); - inFile = false; - } - - sb.append(MessageFormat.format("
", line)).append(line).append("
"); - sb.append("
"); - sb.append(""); - inFile = true; - } else { - boolean gitLinkDiff = line.length() > 0 && line.substring(1).startsWith("Subproject commit"); - if (gitLinkDiff) { - sb.append(""); - if (line.charAt(0) == '+') { - sb.append(""); - } - } - } - sb.append("
"); - } else { - sb.append("
"); - } - } - sb.append(line); - if (gitLinkDiff) { - sb.append("
"); - return sb.toString(); - } - - public DiffStat getDiffStat() { - return diffStat; - } -} +/* + * Copyright 2011 gitblit.com. + * + * 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.gitblit.utils; + +import static org.eclipse.jgit.lib.Constants.encode; +import static org.eclipse.jgit.lib.Constants.encodeASCII; + +import java.io.IOException; +import java.text.MessageFormat; +import java.util.HashMap; +import java.util.Map; + +import org.apache.wicket.Application; +import org.apache.wicket.Localizer; +import org.eclipse.jgit.diff.DiffEntry; +import org.eclipse.jgit.diff.DiffEntry.ChangeType; +import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.diff.RawText; +import org.eclipse.jgit.util.RawParseUtils; + +import com.gitblit.models.PathModel.PathChangeModel; +import com.gitblit.utils.DiffUtils.DiffStat; +import com.gitblit.wicket.GitBlitWebApp; + +/** + * Generates an html snippet of a diff in Gitblit's style, tracks changed paths, and calculates diff stats. + * + * @author James Moger + * @author Tom + * + */ +public class GitBlitDiffFormatter extends DiffFormatter { + + /** + * gitblit.properties key for the per-file limit on the number of diff lines. + */ + private static final String DIFF_LIMIT_PER_FILE_KEY = "web.maxDiffLinesPerFile"; + + /** + * gitblit.properties key for the global limit on the number of diff lines in a commitdiff. + */ + private static final String GLOBAL_DIFF_LIMIT_KEY = "web.maxDiffLines"; + + /** + * Diffs with more lines are not shown in commitdiffs. (Similar to what GitHub does.) Can be reduced + * (but not increased) through gitblit.properties key {@link #DIFF_LIMIT_PER_FILE_KEY}. + */ + private static final int DIFF_LIMIT_PER_FILE = 4000; + + /** + * Global diff limit. Commitdiffs with more lines are truncated. Can be reduced (but not increased) + * through gitblit.properties key {@link #GLOBAL_DIFF_LIMIT_KEY}. + */ + private static final int GLOBAL_DIFF_LIMIT = 20000; + + private final ResettableByteArrayOutputStream os; + + private final DiffStat diffStat; + + private PathChangeModel currentPath; + + private int left, right; + + /** + * If a single file diff in a commitdiff produces more than this number of lines, we don't display + * the diff. First, it's too taxing on the browser: it'll spend an awful lot of time applying the + * CSS rules (despite my having optimized them). And second, no human can read a diff with thousands + * of lines and make sense of it. + *

+ * Set to {@link #DIFF_LIMIT_PER_FILE} for commitdiffs, and to -1 (switches off the limit) for + * single-file diffs. + *

+ */ + private final int maxDiffLinesPerFile; + + /** + * Global limit on the number of diff lines. Set to {@link #GLOBAL_DIFF_LIMIT} for commitdiffs, and + * to -1 (switched off the limit) for single-file diffs. + */ + private final int globalDiffLimit; + + /** Number of lines for the current file diff. Set to zero when a new DiffEntry is started. */ + private int nofLinesCurrent; + /** + * Position in the stream when we try to write the first line. Used to rewind when we detect that + * the diff is too large. + */ + private int startCurrent; + /** Flag set to true when we rewind. Reset to false when we start a new DiffEntry. */ + private boolean isOff; + /** The current diff entry. */ + private DiffEntry entry; + + // Global limit stuff. + + /** Total number of lines written before the current diff entry. */ + private int totalNofLinesPrevious; + /** Running total of the number of diff lines written. Updated until we exceed the global limit. */ + private int totalNofLinesCurrent; + /** Stream position to reset to if we decided to truncate the commitdiff. */ + private int truncateTo; + /** Whether we decided to truncate the commitdiff. */ + private boolean truncated; + /** If {@link #truncated}, contains all files skipped,possibly with a suffix message as value to be displayed. */ + private final Map skipped = new HashMap(); + + public GitBlitDiffFormatter(String commitId, String path) { + super(new ResettableByteArrayOutputStream()); + this.os = (ResettableByteArrayOutputStream) getOutputStream(); + 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. + maxDiffLinesPerFile = path != null ? -1 : getLimit(DIFF_LIMIT_PER_FILE_KEY, 500, DIFF_LIMIT_PER_FILE); + globalDiffLimit = path != null ? -1 : getLimit(GLOBAL_DIFF_LIMIT_KEY, 1000, GLOBAL_DIFF_LIMIT); + } + + /** + * Determines a limit to use for HTML diff output. + * + * @param key + * to use to read the value from the GitBlit settings, if available. + * @param minimum + * minimum value to enforce + * @param maximum + * maximum (and default) value to enforce + * @return the limit + */ + private int getLimit(String key, int minimum, int maximum) { + if (Application.exists()) { + Application application = Application.get(); + if (application instanceof GitBlitWebApp) { + GitBlitWebApp webApp = (GitBlitWebApp) application; + int configValue = webApp.settings().getInteger(key, maximum); + if (configValue < minimum) { + return minimum; + } else if (configValue < maximum) { + return configValue; + } + } + } + return maximum; + } + + /** + * Returns a localized message string, if there is a localization; otherwise the given default value. + * + * @param key + * message key for the message + * @param defaultValue + * to use if no localization for the message can be found + * @return the possibly localized message + */ + private String getMsg(String key, String defaultValue) { + if (Application.exists()) { + Localizer localizer = Application.get().getResourceSettings().getLocalizer(); + if (localizer != null) { + // Use getStringIgnoreSettings because we don't want exceptions here if the key is missing! + return localizer.getStringIgnoreSettings(key, null, null, defaultValue); + } + } + return defaultValue; + } + + @Override + public void format(DiffEntry ent) throws IOException { + currentPath = diffStat.addPath(ent); + nofLinesCurrent = 0; + isOff = false; + entry = ent; + if (!truncated) { + totalNofLinesPrevious = totalNofLinesCurrent; + if (globalDiffLimit > 0 && totalNofLinesPrevious > globalDiffLimit) { + truncated = true; + isOff = true; + } + truncateTo = os.size(); + } else { + isOff = true; + } + if (isOff) { + if (ent.getChangeType().equals(ChangeType.DELETE)) { + skipped.put(ent.getOldPath(), getMsg("gb.diffDeletedFileSkipped", "(deleted file)")); + } else { + skipped.put(ent.getNewPath(), null); + } + } + // Keep formatting, but if off, don't produce anything anymore. We just keep on counting. + super.format(ent); + } + + @Override + public void flush() throws IOException { + if (truncated) { + os.resetTo(truncateTo); + } + super.flush(); + } + + /** + * Rewind and issue a message that the diff is too large. + */ + 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 + } + totalNofLinesCurrent = totalNofLinesPrevious; + isOff = true; + } + } + + /** + * Writes an initial table row containing information about added/removed/renamed/copied files. In case + * of a deletion, we also suppress generating the diff; it's not interesting. (All lines removed.) + */ + private void handleChange() { + // XXX Would be nice if we could generate blob links for the cases handled here. Alas, we lack the repo + // name, and cannot reliably determine it here. We could get the .git directory of a Repository, if we + // passed in the repo, and then take the name of the parent directory, but that'd fail for repos nested + // in GitBlit projects. And we don't know if the repo is inside a project or is a top-level repo. + // + // That's certainly solvable (just pass along more information), but would require a larger rewrite than + // I'm prepared to do now. + String message; + switch (entry.getChangeType()) { + case ADD: + message = getMsg("gb.diffNewFile", "New file"); + break; + case DELETE: + message = getMsg("gb.diffDeletedFile", "File was deleted"); + isOff = true; + break; + case RENAME: + message = MessageFormat.format(getMsg("gb.diffRenamedFile", "File was renamed from {0}"), entry.getOldPath()); + break; + case COPY: + message = MessageFormat.format(getMsg("gb.diffCopiedFile", "File was copied from {0}"), entry.getOldPath()); + break; + 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 + } + } + + /** + * Output a hunk header + * + * @param aStartLine + * within first source + * @param aEndLine + * within first source + * @param bStartLine + * within second source + * @param bEndLine + * within second source + * @throws IOException + */ + @Override + protected void writeHunkHeader(int aStartLine, int aEndLine, int bStartLine, int bEndLine) throws IOException { + if (nofLinesCurrent++ == 0) { + handleChange(); + startCurrent = os.size(); + } + if (!isOff) { + totalNofLinesCurrent++; + if (nofLinesCurrent > maxDiffLinesPerFile && maxDiffLinesPerFile > 0) { + reset(); + } else { + os.write("" + .getBytes()); + os.write('@'); + os.write('@'); + writeRange('-', aStartLine + 1, aEndLine - aStartLine); + writeRange('+', bStartLine + 1, bEndLine - bStartLine); + os.write(' '); + os.write('@'); + os.write('@'); + os.write("\n".getBytes()); + } + } + left = aStartLine + 1; + right = bStartLine + 1; + } + + protected void writeRange(final char prefix, final int begin, final int cnt) throws IOException { + os.write(' '); + os.write(prefix); + switch (cnt) { + case 0: + // If the range is empty, its beginning number must be the + // line just before the range, or 0 if the range is at the + // start of the file stream. Here, begin is always 1 based, + // so an empty file would produce "0,0". + // + os.write(encodeASCII(begin - 1)); + os.write(','); + os.write('0'); + break; + + case 1: + // If the range is exactly one line, produce only the number. + // + os.write(encodeASCII(begin)); + break; + + default: + os.write(encodeASCII(begin)); + os.write(','); + os.write(encodeASCII(cnt)); + break; + } + } + + @Override + protected void writeLine(final char prefix, final RawText text, final int cur) throws IOException { + if (nofLinesCurrent++ == 0) { + handleChange(); + startCurrent = os.size(); + } + // update entry diffstat + currentPath.update(prefix); + if (isOff) { + return; + } + totalNofLinesCurrent++; + if (nofLinesCurrent > maxDiffLinesPerFile && maxDiffLinesPerFile > 0) { + reset(); + } else { + // output diff + os.write("".getBytes()); + switch (prefix) { + case '+': + os.write(("").getBytes()); + os.write("".getBytes()); + os.write("".getBytes()); + break; + case '-': + os.write(("").getBytes()); + os.write("".getBytes()); + os.write("".getBytes()); + break; + default: + os.write(("").getBytes()); + os.write("".getBytes()); + os.write("".getBytes()); + break; + } + String line = text.getString(cur); + line = StringUtils.escapeForHtml(line, false); + os.write(encode(line)); + os.write("\n".getBytes()); + } + } + + /** + * Workaround function for complex private methods in DiffFormatter. This sets the html for the diff headers. + * + * @return + */ + public String getHtml() { + String html = RawParseUtils.decode(os.toByteArray()); + String[] lines = html.split("\n"); + StringBuilder sb = new StringBuilder(); + boolean inFile = false; + String oldnull = "a/dev/null"; + for (String line : lines) { + if (line.startsWith("index")) { + // skip index lines + } else if (line.startsWith("new file") || line.startsWith("deleted file")) { + // skip new file lines + } else if (line.startsWith("\\ No newline")) { + // skip no new line + } else if (line.startsWith("---") || line.startsWith("+++")) { + // skip --- +++ lines + } else if (line.startsWith("diff")) { + line = StringUtils.convertOctal(line); + if (line.indexOf(oldnull) > -1) { + // a is null, use b + line = line.substring(("diff --git " + oldnull).length()).trim(); + // trim b/ + line = line.substring(2).trim(); + } else { + // use a + line = line.substring("diff --git ".length()).trim(); + line = line.substring(line.startsWith("\"a/") ? 3 : 2); + line = line.substring(0, line.indexOf(" b/") > -1 ? line.indexOf(" b/") : line.indexOf("\"b/")).trim(); + } + + if (line.charAt(0) == '"') { + line = line.substring(1); + } + if (line.charAt(line.length() - 1) == '"') { + line = line.substring(0, line.length() - 1); + } + if (inFile) { + sb.append("\n"); + inFile = false; + } + + sb.append(MessageFormat.format("
", line)).append(line) + .append("
"); + sb.append("
"); + sb.append(""); + inFile = true; + } else { + boolean gitLinkDiff = line.length() > 0 && line.substring(1).startsWith("Subproject commit"); + if (gitLinkDiff) { + sb.append(""); + if (line.charAt(0) == '+') { + sb.append(""); + } + } + } + sb.append("
"); + } else { + sb.append(""); + } + } + sb.append(line); + if (gitLinkDiff) { + sb.append("
"); + if (truncated) { + sb.append(MessageFormat.format("
{0}
", + StringUtils.escapeForHtml(getMsg("gb.diffTruncated", "Diff truncated after the above file"), false))); + // List all files not shown. We can be sure we do have at least one path in skipped. + sb.append("
"); + boolean first = true; + for (Map.Entry s : skipped.entrySet()) { + if (!first) { + sb.append('\n'); + } + String path = StringUtils.escapeForHtml(s.getKey(), false); + String comment = s.getValue(); + if (comment != null) { + sb.append("" + path + ' ' + StringUtils.escapeForHtml(comment, false) + ""); + } else { + sb.append("" + path + ""); + } + first = false; + } + sb.append("
"); + } + return sb.toString(); + } + + public DiffStat getDiffStat() { + return diffStat; + } +} diff --git a/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java b/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java new file mode 100644 index 00000000..3a2553f4 --- /dev/null +++ b/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java @@ -0,0 +1,42 @@ +// Copyright (C) 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.package com.gitblit.utils; +package com.gitblit.utils; + +import java.io.ByteArrayOutputStream; + +/** + * A {@link ByteArrayOutputStream} that can be reset to a specified position. + * + * @author Tom + */ +public class ResettableByteArrayOutputStream extends ByteArrayOutputStream { + + /** + * Reset the stream to the given position. If {@code mark} is <= 0, see {@link #reset()}. + * A no-op if the stream contains less than {@code mark} bytes. Otherwise, resets the + * current writing position to {@code mark}. Previously allocated buffer space will be + * reused in subsequent writes. + * + * @param mark + * to set the current writing position to. + */ + public synchronized void resetTo(int mark) { + if (mark <= 0) { + reset(); + } else if (mark < count) { + count = mark; + } + } + +} diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties index ce7b5229..cfec39e5 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties @@ -749,4 +749,11 @@ gb.sortHighestPriority = highest priority gb.sortLowestPriority = lowest priority gb.sortHighestSeverity = highest severity gb.sortLowestSeverity = lowest severity -gb.missingIntegrationBranchMore = The target integration branch does not exist in the repository! \ No newline at end of file +gb.missingIntegrationBranchMore = The target integration branch does not exist in the repository! +gb.diffDeletedFileSkipped = (deleted file) +gb.diffFileDiffTooLarge = Diff too large +gb.diffNewFile = New file +gb.diffDeletedFile = File was deleted +gb.diffRenamedFile = File was renamed from {0} +gb.diffCopiedFile = File was copied from {0} +gb.diffTruncated = Diff truncated after the above file diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties index 3ec330b7..da437c7f 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties @@ -743,3 +743,10 @@ gb.permission = Berechtigung gb.sshKeyPermissionDescription = Geben Sie die Zugriffberechtigung f\u00fcr den SSH Key an gb.transportPreference = \u00dcbertragungseinstellungen gb.transportPreferenceDescription = Geben Sie die \u00dcbertragungsart an, die Sie f\u00fcr das Klonen bevorzugen +gb.diffDeletedFileSkipped = (gel\u00f6schte Datei) +gb.diffFileDiffTooLarge = Zu viele \u00c4nderungen; Diff wird nicht angezeigt +gb.diffNewFile = Neue Datei +gb.diffDeletedFile = Datei wurde gel\u00f6scht +gb.diffRenamedFile = Datei umbenannt von {0} +gb.diffCopiedFile = Datei kopiert von {0} +gb.diffTruncated = Diff nach obiger Datei abgeschnitten diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties index b888beee..67855a0e 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties @@ -672,3 +672,10 @@ gb.ticketIsClosed = Ce ticket est clos. gb.mergeToDescription = branche d'int\u00e9gration par d\u00e9faut pour fusionner les correctifs li\u00e9s aux tickets gb.myTickets = mes tickets gb.yourAssignedTickets = dont vous \u00eates responsable +gb.diffDeletedFileSkipped = (fichier effac\u00e9) +gb.diffFileDiffTooLarge = Trop de diff\u00e9rences, affichage supprim\u00e9e +gb.diffNewFile = Nouveau fichier +gb.diffDeletedFile = Fichier a \u00e9t\u00e9 effac\u00e9 +gb.diffRenamedFile = Fichier renomm\u00e9 de {0} +gb.diffCopiedFile = Fichier copi\u00e9 de {0} +gb.diffTruncated = Affichage de diff\u00e9rences supprim\u00e9e apr\u00e8s le fichier ci-dessus diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index 2484b0cb..43b1be86 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -1350,19 +1350,6 @@ span.diff.unchanged { font-family: inherit; } -div.diff.hunk_header { - -moz-border-bottom-colors: none; - -moz-border-image: none; - -moz-border-left-colors: none; - -moz-border-right-colors: none; - -moz-border-top-colors: none; - border-color: #FFE0FF; - border-style: dotted; - border-width: 1px 0 0; - margin-top: 2px; - font-family: inherit; -} - span.diff.hunk_info { background-color: #FFEEFF; color: #990099; @@ -1374,62 +1361,74 @@ span.diff.hunk_section { font-family: inherit; } -div.diff.add2 { - background-color: #DDFFDD; - font-family: inherit; +.diff-cell { + margin: 0px; + padding: 0px; + border: 0; + border-left: 1px solid #bbb; +} + +.add2 { + background-color: #DDFFDD; } -div.diff.remove2 { +.remove2 { background-color: #FFDDDD; - font-family: inherit; } -div.diff table { +.context2 { + background-color: #fbfbfb; +} + +div.diff > table { border-radius: 0; border-right: 1px solid #bbb; border-bottom: 1px solid #bbb; width: 100%; } -div.diff table th, div.diff table td { - margin: 0px; - padding: 0px; - font-family: monospace; - border: 0; +.diff-line { + background-color: #f0f0f0; + text-align: center; + color: #999; + padding-left: 2px; + padding-right: 2px; + width: 3em; /* Font-size relative! */ +} + +.diff-line:before { + content: attr(data-lineno); } -div.diff table th { +.diff-state { background-color: #f0f0f0; text-align: center; color: #999; - padding-left: 5px; - padding-right: 5px; - width: 30px; + padding-left: 2px; + padding-right: 2px; + width: 0.5em; /* Font-size relative! */ } -div.diff table th.header { - background-color: #D2C3AF; - border-right: 0px; - border-bottom: 1px solid #808080; - font-family: inherit; - font-size:0.9em; - color: black; - padding: 2px; - text-align: left; +.diff-state-add:before { + color: green; + font-weight: bold; + content: '+'; +} + +.diff-state-sub:before { + color: red; + font-weight: bold; + content: '-'; } -div.diff table td.hunk_header { +.hunk_header { background-color: #dAe2e5 !important; + border-left: 1px solid #bbb; border-top: 1px solid #bac2c5; border-bottom: 1px solid #bac2c5; color: #555; } -div.diff table td { - border-left: 1px solid #bbb; - background-color: #fbfbfb; -} - td.changeType { width: 15px; } -- cgit v1.2.3 From 6235fa445c346d54bde35e28a0c0adf0753f06e9 Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 28 Oct 2014 11:01:50 +0100 Subject: Further diff improvements - Add the new settings to gitblit.properties - Highlight trailing whitespace --- src/main/distrib/data/defaults.properties | 26 +++++++++++ .../com/gitblit/utils/GitBlitDiffFormatter.java | 52 +++++++++++++++++----- src/main/resources/gitblit.css | 8 ++++ 3 files changed, 74 insertions(+), 12 deletions(-) diff --git a/src/main/distrib/data/defaults.properties b/src/main/distrib/data/defaults.properties index 5258a3c2..dae2e619 100644 --- a/src/main/distrib/data/defaults.properties +++ b/src/main/distrib/data/defaults.properties @@ -1354,6 +1354,32 @@ web.debugMode = false # SINCE 1.3.0 web.forceDefaultLocale = +# The following two settings serve to avoid browser overload when trying to +# render very large diffs. Both limits apply to commitdiffs, not to single-file +# diffs. + +# Maximum number of diff lines to display for a single file diff in a commitdiff. +# Defaults to 4000; can be adjusted in the range [500 .. 4000]. Smaller values +# set the limit to 500, larger values to 4000. The count includes context lines +# in the diff. +# +# If a file diff in a commitdiff produces more lines, the diff for that file is +# not shown in the commitdiff. +# +# SINCE 1.7.0 +web.maxDiffLinesPerFile = 4000 + +# Total maximum number of diff lines to show in a commitdiff. Defaults to 20000; +# can be adjusted in the range [1000 .. 20000]. Smaller values set the limit to +# 1000, larger values to 20000. The count includes context lines in diffs. +# +# If a commitdiff produces more lines, it is truncated after the first file +# that exceeds the limit. Diffs for subsequent files in the commit are not shown +# at all in the commitdiff. Omitted files are listed, though. +# +# SINCE 1.7.0 +web.maxDiffLines = 20000 + # Enable/disable global regex substitutions (i.e. shared across repositories) # # SINCE 0.5.0 diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index 0b16c374..a1751d22 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -22,6 +22,8 @@ import java.io.IOException; import java.text.MessageFormat; import java.util.HashMap; import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import org.apache.wicket.Application; import org.apache.wicket.Localizer; @@ -37,13 +39,16 @@ import com.gitblit.wicket.GitBlitWebApp; /** * Generates an html snippet of a diff in Gitblit's style, tracks changed paths, and calculates diff stats. - * + * * @author James Moger * @author Tom - * + * */ public class GitBlitDiffFormatter extends DiffFormatter { + /** Regex pattern identifying trailing whitespace. */ + private static final Pattern trailingWhitespace = Pattern.compile("(\\s+?)\r?\n?$"); + /** * gitblit.properties key for the per-file limit on the number of diff lines. */ @@ -129,7 +134,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { /** * Determines a limit to use for HTML diff output. - * + * * @param key * to use to read the value from the GitBlit settings, if available. * @param minimum @@ -156,7 +161,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { /** * Returns a localized message string, if there is a localization; otherwise the given default value. - * + * * @param key * message key for the message * @param defaultValue @@ -268,7 +273,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { /** * Output a hunk header - * + * * @param aStartLine * within first source * @param aEndLine @@ -369,16 +374,38 @@ public class GitBlitDiffFormatter extends DiffFormatter { os.write("".getBytes()); break; } - String line = text.getString(cur); - line = StringUtils.escapeForHtml(line, false); - os.write(encode(line)); + os.write(encode(codeLineToHtml(prefix, text.getString(cur)))); os.write("\n".getBytes()); } } + /** + * Convert the given code line to HTML. + * + * @param prefix + * the diff prefix (+/-) indicating whether the line was added or removed. + * @param line + * the line to format as HTML + * @return the HTML-formatted line, safe for inserting as is into HTML. + */ + private String codeLineToHtml(final char prefix, final String line) { + if ((prefix == '+' || prefix == '-')) { + // Highlight trailing whitespace on deleted/added lines. + Matcher matcher = trailingWhitespace.matcher(line); + if (matcher.find()) { + StringBuilder result = new StringBuilder(StringUtils.escapeForHtml(line.substring(0, matcher.start()), false)); + result.append(""); + result.append(StringUtils.escapeForHtml(matcher.group(1), false)); + result.append(""); + return result.toString(); + } + } + return StringUtils.escapeForHtml(line, false); + } + /** * Workaround function for complex private methods in DiffFormatter. This sets the html for the diff headers. - * + * * @return */ public String getHtml() { @@ -420,7 +447,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { sb.append("\n"); inFile = false; } - + line = StringUtils.escapeForHtml(line, false); sb.append(MessageFormat.format("
", line)).append(line) .append("
"); sb.append("
"); @@ -435,6 +462,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { } else { sb.append(""); } + line = StringUtils.escapeForHtml(line.substring(1), false); } sb.append(line); if (gitLinkDiff) { @@ -456,9 +484,9 @@ public class GitBlitDiffFormatter extends DiffFormatter { String path = StringUtils.escapeForHtml(s.getKey(), false); String comment = s.getValue(); if (comment != null) { - sb.append("" + path + ' ' + StringUtils.escapeForHtml(comment, false) + ""); + sb.append("" + path + ' ' + StringUtils.escapeForHtml(comment, false) + ""); } else { - sb.append("" + path + ""); + sb.append("" + path + ""); } first = false; } diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index 43b1be86..f3982449 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -1380,6 +1380,14 @@ span.diff.hunk_section { background-color: #fbfbfb; } +.trailingws-add { + background-color: #99FF99; +} + +.trailingws-sub { + background-color: #FF9999; +} + div.diff > table { border-radius: 0; border-right: 1px solid #bbb; -- cgit v1.2.3 From f0ebfe6152959fda74bf7a5436bd765a0a231f6b Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 28 Oct 2014 21:55:21 +0100 Subject: More diff page improvements - Use git object ids as fragments and HTML element ids - Simplify generation: don't parse the diff line, instead generate the table header from the DiffEntry when we process it, and just skip the diff lines. --- .../com/gitblit/utils/GitBlitDiffFormatter.java | 76 +++++++++------------- .../com/gitblit/wicket/GitBlitWebApp.properties | 2 +- .../com/gitblit/wicket/GitBlitWebApp_de.properties | 2 +- .../com/gitblit/wicket/GitBlitWebApp_fr.properties | 2 +- .../com/gitblit/wicket/pages/CommitDiffPage.java | 4 +- .../java/com/gitblit/wicket/pages/ComparePage.java | 4 +- 6 files changed, 37 insertions(+), 53 deletions(-) diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index a1751d22..b18093ff 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -20,8 +20,8 @@ import static org.eclipse.jgit.lib.Constants.encodeASCII; import java.io.IOException; import java.text.MessageFormat; -import java.util.HashMap; -import java.util.Map; +import java.util.ArrayList; +import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -119,8 +119,8 @@ public class GitBlitDiffFormatter extends DiffFormatter { private int truncateTo; /** Whether we decided to truncate the commitdiff. */ private boolean truncated; - /** If {@link #truncated}, contains all files skipped,possibly with a suffix message as value to be displayed. */ - private final Map skipped = new HashMap(); + /** If {@link #truncated}, contains all entries skipped. */ + private final List skipped = new ArrayList(); public GitBlitDiffFormatter(String commitId, String path) { super(new ResettableByteArrayOutputStream()); @@ -195,15 +195,30 @@ public class GitBlitDiffFormatter extends DiffFormatter { } else { isOff = true; } - if (isOff) { - if (ent.getChangeType().equals(ChangeType.DELETE)) { - skipped.put(ent.getOldPath(), getMsg("gb.diffDeletedFileSkipped", "(deleted file)")); + if (truncated) { + skipped.add(ent); + } else { + // Produce a header here and now + String path; + String id; + if (ChangeType.DELETE.equals(ent.getChangeType())) { + path = ent.getOldPath(); + id = ent.getOldId().name(); } else { - skipped.put(ent.getNewPath(), null); + path = ent.getNewPath(); + id = ent.getNewId().name(); } + StringBuilder sb = new StringBuilder(MessageFormat.format("
", id)); + sb.append(StringUtils.escapeForHtml(path, false)).append("
"); + sb.append("
\n"); + os.write(sb.toString().getBytes()); } // Keep formatting, but if off, don't produce anything anymore. We just keep on counting. super.format(ent); + if (!truncated) { + // Close the table + os.write("

\n".getBytes()); + } } @Override @@ -412,8 +427,6 @@ public class GitBlitDiffFormatter extends DiffFormatter { String html = RawParseUtils.decode(os.toByteArray()); String[] lines = html.split("\n"); StringBuilder sb = new StringBuilder(); - boolean inFile = false; - String oldnull = "a/dev/null"; for (String line : lines) { if (line.startsWith("index")) { // skip index lines @@ -424,35 +437,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { } else if (line.startsWith("---") || line.startsWith("+++")) { // skip --- +++ lines } else if (line.startsWith("diff")) { - line = StringUtils.convertOctal(line); - if (line.indexOf(oldnull) > -1) { - // a is null, use b - line = line.substring(("diff --git " + oldnull).length()).trim(); - // trim b/ - line = line.substring(2).trim(); - } else { - // use a - line = line.substring("diff --git ".length()).trim(); - line = line.substring(line.startsWith("\"a/") ? 3 : 2); - line = line.substring(0, line.indexOf(" b/") > -1 ? line.indexOf(" b/") : line.indexOf("\"b/")).trim(); - } - - if (line.charAt(0) == '"') { - line = line.substring(1); - } - if (line.charAt(line.length() - 1) == '"') { - line = line.substring(0, line.length() - 1); - } - if (inFile) { - sb.append("
\n"); - inFile = false; - } - line = StringUtils.escapeForHtml(line, false); - sb.append(MessageFormat.format("
", line)).append(line) - .append("
"); - sb.append("
"); - sb.append(""); - inFile = true; + // skip diff lines } else { boolean gitLinkDiff = line.length() > 0 && line.substring(1).startsWith("Subproject commit"); if (gitLinkDiff) { @@ -470,26 +455,25 @@ public class GitBlitDiffFormatter extends DiffFormatter { } } } - sb.append("
"); if (truncated) { sb.append(MessageFormat.format("
{0}
", StringUtils.escapeForHtml(getMsg("gb.diffTruncated", "Diff truncated after the above file"), false))); // List all files not shown. We can be sure we do have at least one path in skipped. sb.append("
"); + String deletedSuffix = StringUtils.escapeForHtml(getMsg("gb.diffDeletedFileSkipped", "(deleted)"), false); boolean first = true; - for (Map.Entry s : skipped.entrySet()) { + for (DiffEntry entry : skipped) { if (!first) { sb.append('\n'); } - String path = StringUtils.escapeForHtml(s.getKey(), false); - String comment = s.getValue(); - if (comment != null) { - sb.append("" + path + ' ' + StringUtils.escapeForHtml(comment, false) + ""); + if (ChangeType.DELETE.equals(entry.getChangeType())) { + sb.append("" + StringUtils.escapeForHtml(entry.getOldPath(), false) + ' ' + deletedSuffix + ""); } else { - sb.append("" + path + ""); + sb.append("" + StringUtils.escapeForHtml(entry.getNewPath(), false) + ""); } first = false; } + skipped.clear(); sb.append("
"); } return sb.toString(); diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties index cfec39e5..c1b5a30e 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp.properties @@ -750,7 +750,7 @@ gb.sortLowestPriority = lowest priority gb.sortHighestSeverity = highest severity gb.sortLowestSeverity = lowest severity gb.missingIntegrationBranchMore = The target integration branch does not exist in the repository! -gb.diffDeletedFileSkipped = (deleted file) +gb.diffDeletedFileSkipped = (deleted) gb.diffFileDiffTooLarge = Diff too large gb.diffNewFile = New file gb.diffDeletedFile = File was deleted diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties index da437c7f..be36ecd1 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_de.properties @@ -743,7 +743,7 @@ gb.permission = Berechtigung gb.sshKeyPermissionDescription = Geben Sie die Zugriffberechtigung f\u00fcr den SSH Key an gb.transportPreference = \u00dcbertragungseinstellungen gb.transportPreferenceDescription = Geben Sie die \u00dcbertragungsart an, die Sie f\u00fcr das Klonen bevorzugen -gb.diffDeletedFileSkipped = (gel\u00f6schte Datei) +gb.diffDeletedFileSkipped = (gel\u00f6scht) gb.diffFileDiffTooLarge = Zu viele \u00c4nderungen; Diff wird nicht angezeigt gb.diffNewFile = Neue Datei gb.diffDeletedFile = Datei wurde gel\u00f6scht diff --git a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties index 67855a0e..1318b1d9 100644 --- a/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties +++ b/src/main/java/com/gitblit/wicket/GitBlitWebApp_fr.properties @@ -672,7 +672,7 @@ gb.ticketIsClosed = Ce ticket est clos. gb.mergeToDescription = branche d'int\u00e9gration par d\u00e9faut pour fusionner les correctifs li\u00e9s aux tickets gb.myTickets = mes tickets gb.yourAssignedTickets = dont vous \u00eates responsable -gb.diffDeletedFileSkipped = (fichier effac\u00e9) +gb.diffDeletedFileSkipped = (effac\u00e9) gb.diffFileDiffTooLarge = Trop de diff\u00e9rences, affichage supprim\u00e9e gb.diffNewFile = Nouveau fichier gb.diffDeletedFile = Fichier a \u00e9t\u00e9 effac\u00e9 diff --git a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java index d827c449..34ff5a29 100644 --- a/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java +++ b/src/main/java/com/gitblit/wicket/pages/CommitDiffPage.java @@ -145,10 +145,10 @@ public class CommitDiffPage extends RepositoryPage { hasSubmodule = submodule.hasSubmodule; // add relative link - item.add(new LinkPanel("pathName", "list", entry.path + " @ " + getShortObjectId(submoduleId), "#" + entry.path)); + item.add(new LinkPanel("pathName", "list", entry.path + " @ " + getShortObjectId(submoduleId), "#n" + entry.objectId)); } else { // add relative link - item.add(new LinkPanel("pathName", "list", entry.path, "#" + entry.path)); + item.add(new LinkPanel("pathName", "list", entry.path, "#n" + entry.objectId)); } // quick links diff --git a/src/main/java/com/gitblit/wicket/pages/ComparePage.java b/src/main/java/com/gitblit/wicket/pages/ComparePage.java index 1ec66133..3b8bb03e 100644 --- a/src/main/java/com/gitblit/wicket/pages/ComparePage.java +++ b/src/main/java/com/gitblit/wicket/pages/ComparePage.java @@ -160,10 +160,10 @@ public class ComparePage extends RepositoryPage { hasSubmodule = submodule.hasSubmodule; // add relative link - item.add(new LinkPanel("pathName", "list", entry.path + " @ " + getShortObjectId(submoduleId), "#" + entry.path)); + item.add(new LinkPanel("pathName", "list", entry.path + " @ " + getShortObjectId(submoduleId), "#n" + entry.objectId)); } else { // add relative link - item.add(new LinkPanel("pathName", "list", entry.path, "#" + entry.path)); + item.add(new LinkPanel("pathName", "list", entry.path, "#n" + entry.objectId)); } // quick links -- cgit v1.2.3 From 599827eff1cedf6141b15f7fdbbcfed4b23ae86a Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 29 Oct 2014 14:50:52 +0100 Subject: CSS changes. - As discussed: - gutter a little lighter, context lines nearly but not quite white. - 2px left (and right) padding in the code column. - I also noticed that somehow all lines were spaced vertically a little wider than on dev.gitblit. Added cellpadding='0' to get the old line height again. --- src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java | 4 ++-- src/main/resources/gitblit.css | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java index b18093ff..5de9e50a 100644 --- a/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java +++ b/src/main/java/com/gitblit/utils/GitBlitDiffFormatter.java @@ -210,7 +210,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { } StringBuilder sb = new StringBuilder(MessageFormat.format("
", id)); sb.append(StringUtils.escapeForHtml(path, false)).append("
"); - sb.append("
\n"); + sb.append("
\n"); os.write(sb.toString().getBytes()); } // Keep formatting, but if off, don't produce anything anymore. We just keep on counting. @@ -459,7 +459,7 @@ public class GitBlitDiffFormatter extends DiffFormatter { sb.append(MessageFormat.format("
{0}
", StringUtils.escapeForHtml(getMsg("gb.diffTruncated", "Diff truncated after the above file"), false))); // List all files not shown. We can be sure we do have at least one path in skipped. - sb.append("
"); + sb.append("
"); String deletedSuffix = StringUtils.escapeForHtml(getMsg("gb.diffDeletedFileSkipped", "(deleted)"), false); boolean first = true; for (DiffEntry entry : skipped) { diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index f3982449..65248086 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -1363,7 +1363,7 @@ span.diff.hunk_section { .diff-cell { margin: 0px; - padding: 0px; + padding: 0 2px; border: 0; border-left: 1px solid #bbb; } @@ -1377,7 +1377,7 @@ span.diff.hunk_section { } .context2 { - background-color: #fbfbfb; + background-color: #FEFEFE; } .trailingws-add { @@ -1396,7 +1396,7 @@ div.diff > table { } .diff-line { - background-color: #f0f0f0; + background-color: #fbfbfb; text-align: center; color: #999; padding-left: 2px; @@ -1409,7 +1409,7 @@ div.diff > table { } .diff-state { - background-color: #f0f0f0; + background-color: #fbfbfb; text-align: center; color: #999; padding-left: 2px; -- cgit v1.2.3 From 8fa8cbcf9db40fea41d7a555c86428aede7f25e8 Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 29 Oct 2014 16:23:04 +0100 Subject: Fix a copy/paste error in a comment. --- .../java/com/gitblit/utils/ResettableByteArrayOutputStream.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java b/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java index 3a2553f4..7df0693f 100644 --- a/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java +++ b/src/main/java/com/gitblit/utils/ResettableByteArrayOutputStream.java @@ -10,14 +10,14 @@ // 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.gitblit.utils; +// limitations under the License. package com.gitblit.utils; import java.io.ByteArrayOutputStream; /** * A {@link ByteArrayOutputStream} that can be reset to a specified position. - * + * * @author Tom */ public class ResettableByteArrayOutputStream extends ByteArrayOutputStream { @@ -27,7 +27,7 @@ public class ResettableByteArrayOutputStream extends ByteArrayOutputStream { * A no-op if the stream contains less than {@code mark} bytes. Otherwise, resets the * current writing position to {@code mark}. Previously allocated buffer space will be * reused in subsequent writes. - * + * * @param mark * to set the current writing position to. */ -- cgit v1.2.3 From 6e55f53a790145dbbe78b8860004e4743e47acf1 Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 6 Nov 2014 22:53:38 +0100 Subject: Add min-width in .diff-line CSS To ensure the line number columns never get squashed. --- src/main/resources/gitblit.css | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/resources/gitblit.css b/src/main/resources/gitblit.css index 65248086..1064231c 100644 --- a/src/main/resources/gitblit.css +++ b/src/main/resources/gitblit.css @@ -1402,6 +1402,7 @@ div.diff > table { padding-left: 2px; padding-right: 2px; width: 3em; /* Font-size relative! */ + min-width: 3em; } .diff-line:before { -- cgit v1.2.3