From f0ebfe6152959fda74bf7a5436bd765a0a231f6b Mon Sep 17 00:00:00 2001 From: Tom Date: Tue, 28 Oct 2014 21:55:21 +0100 Subject: [PATCH] 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. --- .../gitblit/utils/GitBlitDiffFormatter.java | 76 ++++++++----------- .../gitblit/wicket/GitBlitWebApp.properties | 2 +- .../wicket/GitBlitWebApp_de.properties | 2 +- .../wicket/GitBlitWebApp_fr.properties | 2 +- .../gitblit/wicket/pages/CommitDiffPage.java | 4 +- .../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 -- 2.39.5