[PATCH 0/2] Better unquoting of git quoted filenames; esc_path to escape pathnames for display

[PATCH 0/2] Better unquoting of git quoted filenames; esc_path to escape pathnames for display

[Jakub Narebski]

This series fixes unquote so that it can deal with alphabetic escape
sequences like '\n' and '\t', and escaped literal characters like '\"'
and '\\', and adds esc_path do deal with the fact that unquoted 
filenames can now contain control characters, not suitable for HTML.
Previous version of unquote was broken in that it did only octal
escape sequence unquoting, and could leave filename only partially
unquoted.

First patch adds minimal esc_path, second improves esc_path to final
version.

Shortlog:
 [PATCH 1/2] gitweb: Better git-unquoting and gitweb-quoting of pathnames
 [PATCH 2/2] gitweb: Use character or octal escape codes (and add span.cntrl) in esc_path

Diffstat:
 gitweb/gitweb.css  |    7 ++++
 gitweb/gitweb.perl |   95 +++++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 83 insertions(+), 19 deletions(-)

-- 
Jakub Narebski

[PATCH 1/2] gitweb: Better git-unquoting and gitweb-quoting of pathnames

[Jakub Narebski]

Extend unquote subroutine, which unquotes quoted and escaped filenames
which git may return, to deal not only with octal char sequence
quoting, but also quoting ordinary characters including '\"' and '\\'
which are respectively quoted '"' and '\', and to deal also with
C escape sequences including '\t' for TAB and '\n' for LF.

Add esc_path subroutine for gitweb quoting and HTML escaping filenames
(currently it does equivalent of ls' --hide-control-chars, which means
showing undisplayable characters (including '\n' and '\t') as '?'
(question mark) character.  Convert gitweb to use esc_path instead of
simply esc_html to print pathnames.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I hope I catched all the places where esc_html should be replaced by
esc_path.

 gitweb/gitweb.perl |   72 ++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 53 insertions(+), 19 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c46629f..32926f9 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -584,12 +584,46 @@ sub esc_html ($;%) {
 	return $str;
 }
 
+# quote control characters and escape filename to HTML
+sub esc_path {
+	my $str = shift;
+	$str = esc_html($str);
+	$str =~ s/[[:cntrl:]]/?/g; # like --hide-control-chars in ls
+	return $str;
+}
+
 # git may return quoted and escaped filenames
 sub unquote {
 	my $str = shift;
+
+	sub unq {
+		my $seq = shift;
+		my %es = ( # character escape codes, aka escape sequences
+			't' => "\t",   # tab            (HT, TAB)
+			'n' => "\n",   # newline        (NL)
+			'r' => "\r",   # return         (CR)
+			'f' => "\f",   # form feed      (FF)
+			'b' => "\b",   # backspace      (BS)
+			'a' => "\a",   # alarm (bell)   (BEL)
+			'e' => "\e",   # escape         (ESC)
+			'v' => "\013", # vertical tab   (VT)
+		);
+
+		if ($seq =~ m/^[0-7]{1,3}$/) {
+			# octal char sequence
+			return chr(oct($seq));
+		} elsif (exists $es{$seq}) {
+			# C escape sequence, aka character escape code
+			return $es{$seq}
+		}
+		# quoted ordinary character
+		return $seq;
+	}
+
 	if ($str =~ m/^"(.*)"$/) {
+		# needs unquoting
 		$str = $1;
-		$str =~ s/\\([0-7]{1,3})/chr(oct($1))/eg;
+		$str =~ s/\\([^0-7]|[0-7]{1,3})/unq($1)/eg;
 	}
 	return $str;
 }
@@ -1520,7 +1554,7 @@ sub git_header_html {
 		if (defined $action) {
 			$title .= "/$action";
 			if (defined $file_name) {
-				$title .= " - " . esc_html($file_name);
+				$title .= " - " . esc_path($file_name);
 				if ($action eq "tree" && $file_name !~ m|/$|) {
 					$title .= "/";
 				}
@@ -1791,20 +1825,20 @@ sub git_print_page_path {
 			$fullname .= ($fullname ? '/' : '') . $dir;
 			print $cgi->a({-href => href(action=>"tree", file_name=>$fullname,
 			                             hash_base=>$hb),
-			              -title => $fullname}, esc_html($dir));
+			              -title => $fullname}, esc_path($dir));
 			print " / ";
 		}
 		if (defined $type && $type eq 'blob') {
 			print $cgi->a({-href => href(action=>"blob_plain", file_name=>$file_name,
 			                             hash_base=>$hb),
-			              -title => $name}, esc_html($basename));
+			              -title => $name}, esc_path($basename));
 		} elsif (defined $type && $type eq 'tree') {
 			print $cgi->a({-href => href(action=>"tree", file_name=>$file_name,
 			                             hash_base=>$hb),
-			              -title => $name}, esc_html($basename));
+			              -title => $name}, esc_path($basename));
 			print " / ";
 		} else {
-			print esc_html($basename);
+			print esc_path($basename);
 		}
 	}
 	print "<br/></div>\n";
@@ -1876,7 +1910,7 @@ sub git_print_tree_entry {
 		print "<td class=\"list\">" .
 			$cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
 			                       file_name=>"$basedir$t->{'name'}", %base_key),
-			        -class => "list"}, esc_html($t->{'name'})) . "</td>\n";
+			        -class => "list"}, esc_path($t->{'name'})) . "</td>\n";
 		print "<td class=\"link\">";
 		print $cgi->a({-href => href(action=>"blob", hash=>$t->{'hash'},
 					     file_name=>"$basedir$t->{'name'}", %base_key)},
@@ -1903,7 +1937,7 @@ sub git_print_tree_entry {
 		print "<td class=\"list\">";
 		print $cgi->a({-href => href(action=>"tree", hash=>$t->{'hash'},
 		                             file_name=>"$basedir$t->{'name'}", %base_key)},
-		              esc_html($t->{'name'}));
+		              esc_path($t->{'name'}));
 		print "</td>\n";
 		print "<td class=\"link\">";
 		print $cgi->a({-href => href(action=>"tree", hash=>$t->{'hash'},
@@ -1968,7 +2002,7 @@ sub git_difftree_body {
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
 			                             hash_base=>$hash, file_name=>$diff{'file'}),
-			              -class => "list"}, esc_html($diff{'file'}));
+			              -class => "list"}, esc_path($diff{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
 			print "<td class=\"link\">";
@@ -1984,7 +2018,7 @@ sub git_difftree_body {
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'from_id'},
 			                             hash_base=>$parent, file_name=>$diff{'file'}),
-			               -class => "list"}, esc_html($diff{'file'}));
+			               -class => "list"}, esc_path($diff{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chng</td>\n";
 			print "<td class=\"link\">";
@@ -2024,7 +2058,7 @@ sub git_difftree_body {
 			print "<td>";
 			print $cgi->a({-href => href(action=>"blob", hash=>$diff{'to_id'},
 			                             hash_base=>$hash, file_name=>$diff{'file'}),
-			              -class => "list"}, esc_html($diff{'file'}));
+			              -class => "list"}, esc_path($diff{'file'}));
 			print "</td>\n";
 			print "<td>$mode_chnge</td>\n";
 			print "<td class=\"link\">";
@@ -2064,11 +2098,11 @@ sub git_difftree_body {
 			print "<td>" .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
 			                             hash=>$diff{'to_id'}, file_name=>$diff{'to_file'}),
-			              -class => "list"}, esc_html($diff{'to_file'})) . "</td>\n" .
+			              -class => "list"}, esc_path($diff{'to_file'})) . "</td>\n" .
 			      "<td><span class=\"file_status $nstatus\">[$nstatus from " .
 			      $cgi->a({-href => href(action=>"blob", hash_base=>$parent,
 			                             hash=>$diff{'from_id'}, file_name=>$diff{'from_file'}),
-			              -class => "list"}, esc_html($diff{'from_file'})) .
+			              -class => "list"}, esc_path($diff{'from_file'})) .
 			      " with " . (int $diff{'similarity'}) . "% similarity$mode_chng]</span></td>\n" .
 			      "<td class=\"link\">";
 			if ($action eq 'commitdiff') {
@@ -2191,7 +2225,7 @@ sub git_patchset_body {
 			$file  ||= $diffinfo->{'file'};
 			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
 			                               hash=>$diffinfo->{'from_id'}, file_name=>$file),
-			                -class => "list"}, esc_html($file));
+			                -class => "list"}, esc_path($file));
 			$patch_line =~ s|a/.*$|a/$file|g;
 			print "<div class=\"diff from_file\">$patch_line</div>\n";
 
@@ -2203,7 +2237,7 @@ sub git_patchset_body {
 			$file  ||= $diffinfo->{'file'};
 			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
 			                               hash=>$diffinfo->{'to_id'}, file_name=>$file),
-			                -class => "list"}, esc_html($file));
+			                -class => "list"}, esc_path($file));
 			$patch_line =~ s|b/.*|b/$file|g;
 			print "<div class=\"diff to_file\">$patch_line</div>\n";
 
@@ -3521,8 +3555,8 @@ sub git_blobdiff {
 
 	} else {
 		while (my $line = <$fd>) {
-			$line =~ s!a/($hash|$hash_parent)!'a/'.esc_html($diffinfo{'from_file'})!eg;
-			$line =~ s!b/($hash|$hash_parent)!'b/'.esc_html($diffinfo{'to_file'})!eg;
+			$line =~ s!a/($hash|$hash_parent)!'a/'.esc_path($diffinfo{'from_file'})!eg;
+			$line =~ s!b/($hash|$hash_parent)!'b/'.esc_path($diffinfo{'to_file'})!eg;
 
 			print $line;
 
@@ -3879,7 +3913,7 @@ sub git_search {
 						print $cgi->a({-href => href(action=>"blob", hash_base=>$co{'id'},
 						                             hash=>$set{'id'}, file_name=>$set{'file'}),
 						              -class => "list"},
-						              "<span class=\"match\">" . esc_html($set{'file'}) . "</span>") .
+						              "<span class=\"match\">" . esc_path($set{'file'}) . "</span>") .
 						      "<br/>\n";
 					}
 					print "</td>\n" .
@@ -4014,7 +4048,7 @@ XML
 			if (!($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/)) {
 				next;
 			}
-			my $file = esc_html(unquote($7));
+			my $file = esc_path(unquote($7));
 			$file = to_utf8($file);
 			print "$file<br/>\n";
 		}
-- 
1.4.3.4

[PATCH 2/2] gitweb: Use character or octal escape codes (and add span.cntrl) in esc_path

[Jakub Narebski]

Instead of simply hiding control characters in esc_path by replacing
them with '?', use Character Escape Codes (CEC) i.e. alphabetic
backslash sequences like those found in C programming language and
many other languages influenced by it, such as Java and Perl.  If
control characted doesn't have corresponding character escape code,
use octal char sequence to escape it.

Additionally use 'span' element with 'cntrl' attribute to mark escaped
control characters. Add style for span.cntrl in the CSS.

Below there is alternative quoting using Unicode Printable
Representation (PR), i.e. replace control characters with appropriate
Unicode Control Pictures U+2400 - U+243F (9216 - 9279), the Unicode
characters reserved for representing control characters when it is
necessary to print or display them:

sub esc_path {
	my $str = shift;
	$str = esc_html($str);
	$str =~ s!([[:cntrl:]])!sprintf('<span class="cntrl">&#%04d;</span>', 0x2400+ord($1))!eg;
	return $str;
}

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The styling (span.cntrl style) isn't best, I agree.

 gitweb/gitweb.css  |    7 +++++++
 gitweb/gitweb.perl |   25 ++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 0eda982..e19e6bc 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -16,6 +16,13 @@ a:hover, a:visited, a:active {
 	color: #880000;
 }
 
+span.cntrl {
+	border: dashed #aaaaaa;
+	border-width: 1px;
+	padding: 0px 2px 0px 2px;
+	margin:  0px 2px 0px 2px;
+}
+
 img.logo {
 	float: right;
 	border-width: 0px;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 32926f9..c9b16b5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -587,8 +587,31 @@ sub esc_html ($;%) {
 # quote control characters and escape filename to HTML
 sub esc_path {
 	my $str = shift;
+
+	sub quot {
+		my $seq = shift;
+		my %es = ( # character escape codes, aka escape sequences
+			"\t" => '\t',   # tab            (HT, TAB)
+			"\n" => '\n',   # newline        (NL)
+			"\r" => '\r',   # return         (CR)
+			"\f" => '\f',   # form feed      (FF)
+			"\b" => '\b',   # backspace      (BS)
+			"\a" => '\a',   # alarm (bell)   (BEL)
+			"\e" => '\e',   # escape         (ESC)
+			"\013" => '\v', # vertical tab   (VT)
+			"\000" => '\0', # null character (NUL)
+		);
+
+		if (exists $es{$seq}) {
+			# C escape sequence, aka character escape code
+			return $es{$seq};
+		}
+		# octal char sequence
+		return sprintf('\%03o', ord($seq));
+	}
+
 	$str = esc_html($str);
-	$str =~ s/[[:cntrl:]]/?/g; # like --hide-control-chars in ls
+	$str =~ s!([[:cntrl:]])!'<span class="cntrl">' . quot($1) . '</span>'!eg;
 	return $str;
 }
 
-- 
1.4.3.4

[PATCH 0/2] gitweb: New improved patchset view

[Jakub Narebski]

This series of patches is based on previous "[PATCH 0/2] Better unquoting
of git quoted filenames; esc_path to escape pathnames for display" series;
actually previous series was send because it is prerequisite of this series.

This series introduces new "feature", namely new commitdiff and blobdiff
view (new patchset view in short) BUT without this feature the accepted
"gitweb: Output also empty patches in "commitdiff" view" is of dubious value,
as it has not much sense without new patchset view.

Without "new improved patchset view" the empty patches are just that:
totally empty. It is new header and especially outputting extended header
information which makes outputting "empty" patches (with no diff)
in "commitdiff" view usefull. By "empty" patches I mean pure type change,
pure rename, pure copy, and type change with rename.


Shortlog:
 [PATCH 1/2] gitweb: New improved patchset view
 [PATCH 2/2] gitweb: New improved formatting of chunk header in diff

Diffstat:
 gitweb/gitweb.css  |   79 +++++++++++++++++---
 gitweb/gitweb.perl |  201 +++++++++++++++++++++++++++++++++++-----------------
 2 files changed, 203 insertions(+), 77 deletions(-)

-- 
Jakub Narebski
ShadeHawk on #git

[PATCH 1/2] gitweb: New improved patchset view

[Jakub Narebski]

Replace "gitweb diff header" with its full sha1 of blobs and replace
it by "git diff" header and extended diff header. Change also somewhat
highlighting of diffs.

Added `file_type_long' subroutine to convert file mode in octal to
file type description (only for file modes which used by git).

Changes:
* "gitweb diff header" which looked for example like below:
    file:_<sha1 before>_ -> file:_<sha1 after>_
  where 'file' is file type and '<sha1>' is full sha1 of blob is
  changed to
    diff --git _a/<file before>_ _b/<file after>_
  In both cases links are visible and use default link style. If file
  is added, a/<file> is not hyperlinked. If file is deleted, b/<file>
  is not hyperlinked.
* there is added "extended diff header", with <path> and <hash>
  hyperlinked (and <hash> shortened to 7 characters), and <mode>
  explained: '<mode>' is extended to '<mode> (<file type description>)',
  where added text is slightly lighter to easy distinguish that it
  was added (and it is difference from git-diff output).
* from-file/to-file two-line header lines have slightly darker color
  than removed/added lines.
* chunk header has now delicate line above for easier finding chunk
  boundary, and top margin of 2px, both barely visible.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |   66 +++++++++++++++----
 gitweb/gitweb.perl |  183 
++++++++++++++++++++++++++++++++++------------------
 2 files changed, 173 insertions(+), 76 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index e19e6bc..974b47f 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -298,40 +298,82 @@ td.mode {
 	font-family: monospace;
 }
 
-div.diff a.list {
+/* styling of diffs (patchsets): commitdiff and blobdiff views */
+div.diff.header,
+div.diff.extended_header {
+	white-space: normal;
+}
+
+div.diff.header {
+	font-weight: bold;
+
+	background-color: #edece6;
+
+	margin-top: 4px;
+	padding: 4px 0px 2px 0px;
+	border: solid #d9d8d1;
+	border-width: 1px 0px 1px 0px;
+}
+
+div.diff.header a.path {
+	text-decoration: underline;
+}
+
+div.diff.extended_header,
+div.diff.extended_header a.path,
+div.diff.extended_header a.hash {
+	color: #777777;
+}
+
+div.diff.extended_header .info {
+	color: #b0b0b0;
+}
+
+div.diff.extended_header {
+	background-color: #f6f5ee;
+	padding: 2px 0px 2px 0px;
+}
+
+div.diff a.path,
+div.diff a.hash {
 	text-decoration: none;
 }
 
-div.diff a.list:hover {
+div.diff a.path:hover,
+div.diff a.hash:hover {
 	text-decoration: underline;
 }
 
-div.diff.to_file a.list,
-div.diff.to_file,
+div.diff.to_file a.path,
+div.diff.to_file {
+	color: #007000;
+}
+
 div.diff.add {
 	color: #008800;
 }
 
-div.diff.from_file a.list,
-div.diff.from_file,
+div.diff.from_file a.path,
+div.diff.from_file {
+	color: #aa0000;
+}
+
 div.diff.rem {
 	color: #cc0000;
 }
 
 div.diff.chunk_header {
 	color: #990099;
+
+	border: dotted #ffe0ff;
+	border-width: 1px 0px 0px 0px;
+	margin-top: 2px;
 }
 
 div.diff.incomplete {
 	color: #cccccc;
 }
 
-div.diff_info {
-	font-family: monospace;
-	color: #000099;
-	background-color: #edece6;
-	font-style: italic;
-}
 
 div.index_include {
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c9b16b5..2cf8e60 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -780,6 +780,32 @@ sub file_type {
 	}
 }
 
+# convert file mode in octal to file type description string
+sub file_type_long {
+	my $mode = shift;
+
+	if ($mode !~ m/^[0-7]+$/) {
+		return $mode;
+	} else {
+		$mode = oct $mode;
+	}
+
+	if (S_ISDIR($mode & S_IFMT)) {
+		return "directory";
+	} elsif (S_ISLNK($mode)) {
+		return "symlink";
+	} elsif (S_ISREG($mode)) {
+		if ($mode & S_IXUSR) {
+			return "executable";
+		} else {
+			return "file";
+		};
+	} else {
+		return "unknown";
+	}
+}
+
+
 ## 
----------------------------------------------------------------------
 ## functions returning short HTML fragments, or transforming HTML 
fragments
 ## which don't beling to other sections
@@ -2166,6 +2192,7 @@ sub git_patchset_body {
 	my $in_header = 0;
 	my $patch_found = 0;
 	my $diffinfo;
+	my (%from, %to);
 
 	print "<div class=\"patchset\">\n";
 
@@ -2176,6 +2203,10 @@ sub git_patchset_body {
 		if ($patch_line =~ m/^diff /) { # "git diff" header
 			# beginning of patch (in patchset)
 			if ($patch_found) {
+				# close extended header for previous empty patch
+				if ($in_header) {
+					print "</div>\n" # class="diff extended_header"
+				}
 				# close previous patch
 				print "</div>\n"; # class="patch"
 			} else {
@@ -2184,89 +2215,113 @@ sub git_patchset_body {
 			}
 			print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
 
+			# read and prepare patch information
 			if (ref($difftree->[$patch_idx]) eq "HASH") {
+				# pre-parsed (or generated by hand)
 				$diffinfo = $difftree->[$patch_idx];
 			} else {
 				$diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
 			}
+			$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+			$to{'file'}   = $diffinfo->{'to_file'}   || $diffinfo->{'file'};
+			if ($diffinfo->{'status'} ne "A") { # not new (added) file
+				$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
+				                     hash=>$diffinfo->{'from_id'},
+				                     file_name=>$from{'file'});
+			}
+			if ($diffinfo->{'status'} ne "D") { # not deleted file
+				$to{'href'} = href(action=>"blob", hash_base=>$hash,
+				                   hash=>$diffinfo->{'to_id'},
+				                   file_name=>$to{'file'});
+			}
 			$patch_idx++;
 
-			if ($diffinfo->{'status'} eq "A") { # added
-				print "<div class=\"diff_info\">" . 
file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'}) . " (new)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "D") { # deleted
-				print "<div class=\"diff_info\">" . 
file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) . " (deleted)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "R" || # renamed
-			         $diffinfo->{'status'} eq "C" || # copied
-			         $diffinfo->{'status'} eq "2") { # with two filenames (from 
git_blobdiff)
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'from_file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'to_file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
-
-			} else { # modified, mode changed, ...
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, 
file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
+			# print "git diff" header
+			$patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
+			if ($from{'href'}) {
+				$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
+				                       'a/' . esc_path($from{'file'}));
+			} else { # file was added
+				$patch_line .= 'a/' . esc_path($from{'file'});
+			}
+			$patch_line .= ' ';
+			if ($to{'href'}) {
+				$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+				                       'b/' . esc_path($to{'file'}));
+			} else { # file was deleted
+				$patch_line .= 'b/' . esc_path($to{'file'});
 			}
 
-			#print "<div class=\"diff extended_header\">\n";
+			print "<div class=\"diff header\">$patch_line</div>\n";
+			print "<div class=\"diff extended_header\">\n";
 			$in_header = 1;
 			next LINE;
-		} # start of patch in patchset
+		}
 
+		if ($in_header) {
+			if ($patch_line !~ m/^---/) {
+				# match <path>
+				if ($patch_line =~ s!^((copy|rename) from ).*$!$1! && 
$from{'href'}) {
+					$patch_line .= $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                        esc_path($from{'file'}));
+				}
+				if ($patch_line =~ s!^((copy|rename) to ).*$!$1! && $to{'href'}) {
+					$patch_line = $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_path($to{'file'}));
+				}
+				# match <mode>
+				if ($patch_line =~ m/\s(\d{6})$/) {
+					$patch_line .= '<span class="info"> (' .
+					               file_type_long($1) .
+					               ')</span>';
+				}
+				# match <hash>
+				if ($patch_line =~ m/^index/) {
+					my ($from_link, $to_link);
+					if ($from{'href'}) {
+						$from_link = $cgi->a({-href=>$from{'href'}, -class=>"hash"},
+						                     substr($diffinfo->{'from_id'},0,7));
+					} else {
+						$from_link = '0' x 7;
+					}
+					if ($to{'href'}) {
+						$to_link = $cgi->a({-href=>$to{'href'}, -class=>"hash"},
+						                   substr($diffinfo->{'to_id'},0,7));
+					} else {
+						$to_link = '0' x 7;
+					}
+					my ($from_id, $to_id) = ($diffinfo->{'from_id'}, 
$diffinfo->{'to_id'});
+					$patch_line =~ s!$from_id\.\.$to_id!$from_link..$to_link!;
+				}
+				print $patch_line . "<br/>\n";
 
-		if ($in_header && $patch_line =~ m/^---/) {
-			#print "</div>\n"; # class="diff extended_header"
-			$in_header = 0;
+			} else {
+				#$in_header && $patch_line =~ m/^---/;
+				print "</div>\n"; # class="diff extended_header"
+				$in_header = 0;
+
+				if ($from{'href'}) {
+					$patch_line = '--- a/' .
+					              $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                      esc_path($from{'file'}));
+				}
+				print "<div class=\"diff from_file\">$patch_line</div>\n";
 
-			my $file = $diffinfo->{'from_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", 
hash_base=>$hash_parent,
-			                               hash=>$diffinfo->{'from_id'}, 
file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|a/.*$|a/$file|g;
-			print "<div class=\"diff from_file\">$patch_line</div>\n";
+				$patch_line = <$fd>;
+				chomp $patch_line;
 
-			$patch_line = <$fd>;
-			chomp $patch_line;
+				#$patch_line =~ m/^+++/;
+				if ($to{'href'}) {
+					$patch_line = '+++ b/' .
+					              $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_path($to{'file'}));
+				}
+				print "<div class=\"diff to_file\">$patch_line</div>\n";
 
-			#$patch_line =~ m/^+++/;
-			$file    = $diffinfo->{'to_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                               hash=>$diffinfo->{'to_id'}, 
file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|b/.*|b/$file|g;
-			print "<div class=\"diff to_file\">$patch_line</div>\n";
+			}
 
 			next LINE;
 		}
-		next LINE if $in_header;
 
 		print format_diff_line($patch_line);
 	}
-- 
1.4.3.4

Re: [PATCH 1/2] gitweb: New improved patchset view

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> ---
>  gitweb/gitweb.css  |   66 +++++++++++++++----
>  gitweb/gitweb.perl |  183 
> ++++++++++++++++++++++++++++++++++------------------
>  2 files changed, 173 insertions(+), 76 deletions(-)

Linewrap?

> ----------------------------------------------------------------------
>  ## functions returning short HTML fragments, or transforming HTML 
> fragments

...

Re: [PATCH 1/2] gitweb: New improved patchset view

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Jakub Narebski <jnareb@gmail.com> writes:
>
>> ---
>>  gitweb/gitweb.css  |   66 +++++++++++++++----
>>  gitweb/gitweb.perl |  183 
>> ++++++++++++++++++++++++++++++++++------------------
>>  2 files changed, 173 insertions(+), 76 deletions(-)
>
> Linewrap?
>
>> ----------------------------------------------------------------------
>>  ## functions returning short HTML fragments, or transforming HTML 
>> fragments
>
> ...

No need to resend -- will manage.

Re: [PATCH 1/2] gitweb: New improved patchset view

[Jakub Narebski]

Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
> > Jakub Narebski <jnareb@gmail.com> writes:
> >
> >> ---
> >>  gitweb/gitweb.css  |   66 +++++++++++++++----
> >>  gitweb/gitweb.perl |  183 
> >> ++++++++++++++++++++++++++++++++++------------------
> >>  2 files changed, 173 insertions(+), 76 deletions(-)
> >
> > Linewrap?
> >
> >> 
----------------------------------------------------------------------
> >>  ## functions returning short HTML fragments, or transforming HTML 
> >> fragments
> >
> > ...
> 
> No need to resend -- will manage.

Too late... ;-)

-- 
Jakub Narebski

[PATCH 1/2 (amend)] gitweb: New improved patchset view

[Jakub Narebski]

Replace "gitweb diff header" with its full sha1 of blobs and replace
it by "git diff" header and extended diff header. Change also somewhat
highlighting of diffs.

Added `file_type_long' subroutine to convert file mode in octal to
file type description (only for file modes which used by git).

Changes:
* "gitweb diff header" which looked for example like below:
    file:_<sha1 before>_ -> file:_<sha1 after>_
  where 'file' is file type and '<sha1>' is full sha1 of blob is
  changed to
    diff --git _a/<file before>_ _b/<file after>_
  In both cases links are visible and use default link style. If file
  is added, a/<file> is not hyperlinked. If file is deleted, b/<file>
  is not hyperlinked.
* there is added "extended diff header", with <path> and <hash>
  hyperlinked (and <hash> shortened to 7 characters), and <mode>
  explained: '<mode>' is extended to '<mode> (<file type description>)',
  where added text is slightly lighter to easy distinguish that it
  was added (and it is difference from git-diff output).
* from-file/to-file two-line header lines have slightly darker color
  than removed/added lines.
* chunk header has now delicate line above for easier finding chunk
  boundary, and top margin of 2px, both barely visible.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
I'm sorry for previous version with linewrap.

 gitweb/gitweb.css  |   66 +++++++++++++++----
 gitweb/gitweb.perl |  183 ++++++++++++++++++++++++++++++++++------------------
 2 files changed, 173 insertions(+), 76 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index e19e6bc..974b47f 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -298,40 +298,82 @@ td.mode {
 	font-family: monospace;
 }
 
-div.diff a.list {
+/* styling of diffs (patchsets): commitdiff and blobdiff views */
+div.diff.header,
+div.diff.extended_header {
+	white-space: normal;
+}
+
+div.diff.header {
+	font-weight: bold;
+
+	background-color: #edece6;
+
+	margin-top: 4px;
+	padding: 4px 0px 2px 0px;
+	border: solid #d9d8d1;
+	border-width: 1px 0px 1px 0px;
+}
+
+div.diff.header a.path {
+	text-decoration: underline;
+}
+
+div.diff.extended_header,
+div.diff.extended_header a.path,
+div.diff.extended_header a.hash {
+	color: #777777;
+}
+
+div.diff.extended_header .info {
+	color: #b0b0b0;
+}
+
+div.diff.extended_header {
+	background-color: #f6f5ee;
+	padding: 2px 0px 2px 0px;
+}
+
+div.diff a.path,
+div.diff a.hash {
 	text-decoration: none;
 }
 
-div.diff a.list:hover {
+div.diff a.path:hover,
+div.diff a.hash:hover {
 	text-decoration: underline;
 }
 
-div.diff.to_file a.list,
-div.diff.to_file,
+div.diff.to_file a.path,
+div.diff.to_file {
+	color: #007000;
+}
+
 div.diff.add {
 	color: #008800;
 }
 
-div.diff.from_file a.list,
-div.diff.from_file,
+div.diff.from_file a.path,
+div.diff.from_file {
+	color: #aa0000;
+}
+
 div.diff.rem {
 	color: #cc0000;
 }
 
 div.diff.chunk_header {
 	color: #990099;
+
+	border: dotted #ffe0ff;
+	border-width: 1px 0px 0px 0px;
+	margin-top: 2px;
 }
 
 div.diff.incomplete {
 	color: #cccccc;
 }
 
-div.diff_info {
-	font-family: monospace;
-	color: #000099;
-	background-color: #edece6;
-	font-style: italic;
-}
 
 div.index_include {
 	border: solid #d9d8d1;
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index c9b16b5..2cf8e60 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -780,6 +780,32 @@ sub file_type {
 	}
 }
 
+# convert file mode in octal to file type description string
+sub file_type_long {
+	my $mode = shift;
+
+	if ($mode !~ m/^[0-7]+$/) {
+		return $mode;
+	} else {
+		$mode = oct $mode;
+	}
+
+	if (S_ISDIR($mode & S_IFMT)) {
+		return "directory";
+	} elsif (S_ISLNK($mode)) {
+		return "symlink";
+	} elsif (S_ISREG($mode)) {
+		if ($mode & S_IXUSR) {
+			return "executable";
+		} else {
+			return "file";
+		};
+	} else {
+		return "unknown";
+	}
+}
+
+
 ## ----------------------------------------------------------------------
 ## functions returning short HTML fragments, or transforming HTML fragments
 ## which don't beling to other sections
@@ -2166,6 +2192,7 @@ sub git_patchset_body {
 	my $in_header = 0;
 	my $patch_found = 0;
 	my $diffinfo;
+	my (%from, %to);
 
 	print "<div class=\"patchset\">\n";
 
@@ -2176,6 +2203,10 @@ sub git_patchset_body {
 		if ($patch_line =~ m/^diff /) { # "git diff" header
 			# beginning of patch (in patchset)
 			if ($patch_found) {
+				# close extended header for previous empty patch
+				if ($in_header) {
+					print "</div>\n" # class="diff extended_header"
+				}
 				# close previous patch
 				print "</div>\n"; # class="patch"
 			} else {
@@ -2184,89 +2215,113 @@ sub git_patchset_body {
 			}
 			print "<div class=\"patch\" id=\"patch". ($patch_idx+1) ."\">\n";
 
+			# read and prepare patch information
 			if (ref($difftree->[$patch_idx]) eq "HASH") {
+				# pre-parsed (or generated by hand)
 				$diffinfo = $difftree->[$patch_idx];
 			} else {
 				$diffinfo = parse_difftree_raw_line($difftree->[$patch_idx]);
 			}
+			$from{'file'} = $diffinfo->{'from_file'} || $diffinfo->{'file'};
+			$to{'file'}   = $diffinfo->{'to_file'}   || $diffinfo->{'file'};
+			if ($diffinfo->{'status'} ne "A") { # not new (added) file
+				$from{'href'} = href(action=>"blob", hash_base=>$hash_parent,
+				                     hash=>$diffinfo->{'from_id'},
+				                     file_name=>$from{'file'});
+			}
+			if ($diffinfo->{'status'} ne "D") { # not deleted file
+				$to{'href'} = href(action=>"blob", hash_base=>$hash,
+				                   hash=>$diffinfo->{'to_id'},
+				                   file_name=>$to{'file'});
+			}
 			$patch_idx++;
 
-			if ($diffinfo->{'status'} eq "A") { # added
-				print "<div class=\"diff_info\">" . file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'}) . " (new)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "D") { # deleted
-				print "<div class=\"diff_info\">" . file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) . " (deleted)" .
-				      "</div>\n"; # class="diff_info"
-
-			} elsif ($diffinfo->{'status'} eq "R" || # renamed
-			         $diffinfo->{'status'} eq "C" || # copied
-			         $diffinfo->{'status'} eq "2") { # with two filenames (from git_blobdiff)
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, file_name=>$diffinfo->{'from_file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, file_name=>$diffinfo->{'to_file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
-
-			} else { # modified, mode changed, ...
-				print "<div class=\"diff_info\">" .
-				      file_type($diffinfo->{'from_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-				                             hash=>$diffinfo->{'from_id'}, file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'from_id'}) .
-				      " -> " .
-				      file_type($diffinfo->{'to_mode'}) . ":" .
-				      $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-				                             hash=>$diffinfo->{'to_id'}, file_name=>$diffinfo->{'file'})},
-				              $diffinfo->{'to_id'});
-				print "</div>\n"; # class="diff_info"
+			# print "git diff" header
+			$patch_line =~ s!^(diff (.*?) )"?a/.*$!$1!;
+			if ($from{'href'}) {
+				$patch_line .= $cgi->a({-href => $from{'href'}, -class => "path"},
+				                       'a/' . esc_path($from{'file'}));
+			} else { # file was added
+				$patch_line .= 'a/' . esc_path($from{'file'});
+			}
+			$patch_line .= ' ';
+			if ($to{'href'}) {
+				$patch_line .= $cgi->a({-href => $to{'href'}, -class => "path"},
+				                       'b/' . esc_path($to{'file'}));
+			} else { # file was deleted
+				$patch_line .= 'b/' . esc_path($to{'file'});
 			}
 
-			#print "<div class=\"diff extended_header\">\n";
+			print "<div class=\"diff header\">$patch_line</div>\n";
+			print "<div class=\"diff extended_header\">\n";
 			$in_header = 1;
 			next LINE;
-		} # start of patch in patchset
+		}
 
+		if ($in_header) {
+			if ($patch_line !~ m/^---/) {
+				# match <path>
+				if ($patch_line =~ s!^((copy|rename) from ).*$!$1! && $from{'href'}) {
+					$patch_line .= $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                        esc_path($from{'file'}));
+				}
+				if ($patch_line =~ s!^((copy|rename) to ).*$!$1! && $to{'href'}) {
+					$patch_line = $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_path($to{'file'}));
+				}
+				# match <mode>
+				if ($patch_line =~ m/\s(\d{6})$/) {
+					$patch_line .= '<span class="info"> (' .
+					               file_type_long($1) .
+					               ')</span>';
+				}
+				# match <hash>
+				if ($patch_line =~ m/^index/) {
+					my ($from_link, $to_link);
+					if ($from{'href'}) {
+						$from_link = $cgi->a({-href=>$from{'href'}, -class=>"hash"},
+						                     substr($diffinfo->{'from_id'},0,7));
+					} else {
+						$from_link = '0' x 7;
+					}
+					if ($to{'href'}) {
+						$to_link = $cgi->a({-href=>$to{'href'}, -class=>"hash"},
+						                   substr($diffinfo->{'to_id'},0,7));
+					} else {
+						$to_link = '0' x 7;
+					}
+					my ($from_id, $to_id) = ($diffinfo->{'from_id'}, $diffinfo->{'to_id'});
+					$patch_line =~ s!$from_id\.\.$to_id!$from_link..$to_link!;
+				}
+				print $patch_line . "<br/>\n";
 
-		if ($in_header && $patch_line =~ m/^---/) {
-			#print "</div>\n"; # class="diff extended_header"
-			$in_header = 0;
+			} else {
+				#$in_header && $patch_line =~ m/^---/;
+				print "</div>\n"; # class="diff extended_header"
+				$in_header = 0;
+
+				if ($from{'href'}) {
+					$patch_line = '--- a/' .
+					              $cgi->a({-href=>$from{'href'}, -class=>"path"},
+					                      esc_path($from{'file'}));
+				}
+				print "<div class=\"diff from_file\">$patch_line</div>\n";
 
-			my $file = $diffinfo->{'from_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash_parent,
-			                               hash=>$diffinfo->{'from_id'}, file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|a/.*$|a/$file|g;
-			print "<div class=\"diff from_file\">$patch_line</div>\n";
+				$patch_line = <$fd>;
+				chomp $patch_line;
 
-			$patch_line = <$fd>;
-			chomp $patch_line;
+				#$patch_line =~ m/^+++/;
+				if ($to{'href'}) {
+					$patch_line = '+++ b/' .
+					              $cgi->a({-href=>$to{'href'}, -class=>"path"},
+					                      esc_path($to{'file'}));
+				}
+				print "<div class=\"diff to_file\">$patch_line</div>\n";
 
-			#$patch_line =~ m/^+++/;
-			$file    = $diffinfo->{'to_file'};
-			$file  ||= $diffinfo->{'file'};
-			$file = $cgi->a({-href => href(action=>"blob", hash_base=>$hash,
-			                               hash=>$diffinfo->{'to_id'}, file_name=>$file),
-			                -class => "list"}, esc_path($file));
-			$patch_line =~ s|b/.*|b/$file|g;
-			print "<div class=\"diff to_file\">$patch_line</div>\n";
+			}
 
 			next LINE;
 		}
-		next LINE if $in_header;
 
 		print format_diff_line($patch_line);
 	}
-- 
1.4.3.4

[PATCH 2/2] gitweb: New improved formatting of chunk header in diff

[Jakub Narebski]

If we have provided enough info, and diff is not combined diff,
and if provided diff line is chunk header, then:
* split chunk header into .chunk_info and .section span elements,
  first containing proper chunk header, second section heading
  (aka. which function), for separate styling: the proper chunk
  header is on non-white background, section heading part uses
  slightly lighter color.
* hyperlink from-file-range to starting line of from-file, if file
  was not created.
* hyperlink to-file-range to starting line of to-file, if file
  was not deleted.
Links are of invisible variety (and "list" class).

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.css  |   13 +++++++++++++
 gitweb/gitweb.perl |   18 +++++++++++++++++-
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 974b47f..7177c6e 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -334,11 +334,13 @@ div.diff.extended_header {
 	padding: 2px 0px 2px 0px;
 }
 
+div.diff a.list,
 div.diff a.path,
 div.diff a.hash {
 	text-decoration: none;
 }
 
+div.diff a.list:hover,
 div.diff a.path:hover,
 div.diff a.hash:hover {
 	text-decoration: underline;
@@ -362,14 +364,25 @@ div.diff.rem {
 	color: #cc0000;
 }
 
+div.diff.chunk_header a,
 div.diff.chunk_header {
 	color: #990099;
+}
 
+div.diff.chunk_header {
 	border: dotted #ffe0ff;
 	border-width: 1px 0px 0px 0px;
 	margin-top: 2px;
 }
 
+div.diff.chunk_header span.chunk_info {
+	background-color: #ffeeff;
+}
+
+div.diff.chunk_header span.section {
+	color: #aa22aa;
+}
+
 div.diff.incomplete {
 	color: #cccccc;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2cf8e60..3118cb0 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -873,6 +873,7 @@ sub format_subject_html {
 
 sub format_diff_line {
 	my $line = shift;
+	my ($from, $to) = @_;
 	my $char = substr($line, 0, 1);
 	my $diff_class = "";
 
@@ -888,6 +889,21 @@ sub format_diff_line {
 		$diff_class = " incomplete";
 	}
 	$line = untabify($line);
+	if ($from && $to && $line =~ m/^\@{2} /) {
+		my ($from_text, $from_start, $from_lines, $to_text, $to_start, $to_lines, $section) =
+			$line =~ m/^\@{2} (-(\d+),(\d+)) (\+(\d+),(\d+)) \@{2}(.*)$/;
+		if ($from->{'href'}) {
+			$from_text = $cgi->a({-href=>"$from->{'href'}#l$from_start",
+			                     -class=>"list"}, $from_text);
+		}
+		if ($to->{'href'}) {
+			$to_text   = $cgi->a({-href=>"$to->{'href'}#l$to_start",
+			                     -class=>"list"}, $to_text);
+		}
+		$line = "<span class=\"chunk_info\">@@ $from_text $to_text @@</span>" .
+		        "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
+		return "<div class=\"diff$diff_class\">$line</div>\n";
+	}
 	return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1) . "</div>\n";
 }
 
@@ -2323,7 +2339,7 @@ sub git_patchset_body {
 			next LINE;
 		}
 
-		print format_diff_line($patch_line);
+		print format_diff_line($patch_line, \%from, \%to);
 	}
 	print "</div>\n" if $patch_found; # class="patch"
 
-- 
1.4.3.4

Re: [PATCH 2/2] gitweb: New improved formatting of chunk header in diff

[Junio C Hamano]

This regresses on hunks of form "@@ -n +m,l @@" and friends, so
I dropped it for now.

I've applied other three patches but with manual tweaks while I
demangled the linewrap, so please sanity check the result after
it mirrors out.  I also noticed that we unconditionally give
blame link in "diff-tree --name-status" part of commitdiff, so
I've fixed it up as well.

I think you still leak control characters in commit messages and
htmlized blob text view; and perhaps others like blame.  I have
made quot_cec into a separate sub that is callable from outside
esc_path, so these places should be able to reuse it to make
control characters "visible and safe" and keep the presentation
consistent.  The non-path callers would not feed tabs into the
function (I haven't looked at the code but I suspect you would
replace them into runs of spaces to the next tab stop by hand),
but other controls need to be quoted.

Re: [PATCH 2/2] gitweb: New improved formatting of chunk header in diff

[Jakub Narebski]

Junio C Hamano wrote:
> This regresses on hunks of form "@@ -n +m,l @@" and friends, so
> I dropped it for now.

O.K. This one introduced new feature, and wasn't needed for earlier 
accepted patch (output empty patches) to have sense. And it is rc1 
phase...

By the way, where I can find proper specifiction of unified diff format? 
Do I understand correctly that bot from and to ranges can be without 
number of lines part if it simplifies to 0?
-- 
Jakub Narebski

Re: [PATCH 2/2] gitweb: New improved formatting of chunk header in diff

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> O.K. This one introduced new feature, and wasn't needed for earlier 
> accepted patch (output empty patches) to have sense. And it is rc1 
> phase...

I personally do not mind minor gitweb 'feature' updates in -rc
cycle, just like I am planning to see if there are gitk updates
I haven't pulled from Paulus (I think there is at least one
patch from last month or so), unless the changes do not break it
so badly.

> By the way, where I can find proper specifiction of unified diff format? 
> Do I understand correctly that bot from and to ranges can be without 
> number of lines part if it simplifies to 0?

When Linus did apply.c and I did diff.c, we primarily worked off
of sources to GNU patch.

There is a POSIX draft proposal now.

http://www.opengroup.org/austin/mailarchives/ag-review/msg02077.html

See also updates about the proposal.

http://thread.gmane.org/gmane.comp.version-control.git/29331/focus=29389

Re: [PATCH 2/2] gitweb: New improved formatting of chunk header in diff

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> O.K. This one introduced new feature, and wasn't needed for earlier 
>> accepted patch (output empty patches) to have sense. And it is rc1 
>> phase...
> 
> I personally do not mind minor gitweb 'feature' updates in -rc
> cycle, just like I am planning to see if there are gitk updates
> I haven't pulled from Paulus (I think there is at least one
> patch from last month or so), unless the changes do not break it
> so badly.

I'll resend corrected patch then...
 
>> By the way, where I can find proper specifiction of unified diff format? 
>> Do I understand correctly that bot from and to ranges can be without 
>> number of lines part if it simplifies to 0?
> 
> When Linus did apply.c and I did diff.c, we primarily worked off
> of sources to GNU patch.
> 
> There is a POSIX draft proposal now.
> 
> http://www.opengroup.org/austin/mailarchives/ag-review/msg02077.html
> 
> See also updates about the proposal.
> 
> http://thread.gmane.org/gmane.comp.version-control.git/29331/focus=29389

Thanks a lot.

Personally I think that we should push for git extended headers to be
(as an option) in POSIX for unifed diff... well, with the exception
of index and similarity lines. ;-)

-- 
Jakub Narebski

Re: [PATCH 2/2] gitweb: Use character or octal escape codes (and add span.cntrl) in esc_path

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Instead of simply hiding control characters in esc_path by replacing
> them with '?', use Character Escape Codes (CEC) i.e. alphabetic
> backslash sequences like those found in C programming language and
> many other languages influenced by it, such as Java and Perl.  If
> control characted doesn't have corresponding character escape code,
> use octal char sequence to escape it.
>
> Additionally use 'span' element with 'cntrl' attribute to mark escaped
> control characters. Add style for span.cntrl in the CSS.

Would have preferred the 'span' thing to be part of [1/2]...

Didn't I ask quot to be freestanding sub, not nested?  That
would make UPR vs CEC experiments easier and more pleasant.

Re: [PATCH 2/2] gitweb: Use character or octal escape codes (and add span.cntrl) in esc_path

[Jakub Narebski]

Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>> Instead of simply hiding control characters in esc_path by replacing
>> them with '?', use Character Escape Codes (CEC) i.e. alphabetic
>> backslash sequences like those found in C programming language and
>> many other languages influenced by it, such as Java and Perl.  If
>> control characted doesn't have corresponding character escape code,
>> use octal char sequence to escape it.
>>
>> Additionally use 'span' element with 'cntrl' attribute to mark escaped
>> control characters. Add style for span.cntrl in the CSS.
> 
> Would have preferred the 'span' thing to be part of [1/2]...

I've misunderstood you then. I'm sorry.

> Didn't I ask quot to be freestanding sub, not nested?  That
> would make UPR vs CEC experiments easier and more pleasant.

In what way having freestanding and not nested sub would "make
UPR vs CEC experiments easier and more pleasant"?

If it is important, I can send patch moving quot sub outside
esc_path (and perhaps also unq sub outside unquote), or send
corrected patch... or you can correct patch by hand...

-- 
Jakub Narebski