Further diff improvements

- Add the new settings to gitblit.properties
- Highlight trailing whitespace

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 <tw201207@gmail.com>
- * 
+ *
  */
 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("<td class='diff-cell context2'>".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("</td></tr>\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("<span class='trailingws-").append(prefix == '+' ? "add" : "sub").append("'>");
+				result.append(StringUtils.escapeForHtml(matcher.group(1), false));
+				result.append("</span>");
+				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("</tbody></table></div>\n");
 					inFile = false;
 				}
-
+				line = StringUtils.escapeForHtml(line, false);
 				sb.append(MessageFormat.format("<div class='header'><div class=\"diffHeader\" id=\"{0}\"><i class=\"icon-file\"></i> ", line)).append(line)
 						.append("</div></div>");
 				sb.append("<div class=\"diff\">");
@@ -435,6 +462,7 @@ public class GitBlitDiffFormatter extends DiffFormatter {
 					} else {
 						sb.append("<th class='diff-state diff-state-sub'></th><td class=\"diff-cell remove2\">");
 					}
+					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("<span id='" + path + "'>" + path + ' ' + StringUtils.escapeForHtml(comment, false) + "</span>");
+					sb.append("<span id=\"" + path + "\">" + path + ' ' + StringUtils.escapeForHtml(comment, false) + "</span>");
 				} else {
-					sb.append("<span id='" + path + "'>" + path + "</span>");
+					sb.append("<span id=\"" + path + "\">" + path + "</span>");
 				}
 				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;