* [PATCHv3 1/8] gitweb: Refactor diff body line classification
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 2/8] gitweb: Extract formatting of diff chunk header Jakub Narebski
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski, Junio C Hamano
Simplify classification of diff line body in format_diff_line(),
replacing two long if-elsif chains (one for ordinary diff and one for
combined diff of a merge commit) with a single regexp match. Refactor
this code into diff_line_class() function.
While at it:
* Fix an artifact in that $diff_class included leading space to be
able to compose classes like this "class=\"diff$diff_class\"', even
when $diff_class was an empty string. This made code unnecessary
ugly: $diff_class is now just class name or an empty string.
* Introduce "ctx" class for context lines ($diff_class was set to ""
in this case before this commit).
Idea and initial code by Junio C Hamano, polish and testing by Jakub
Narebski. Inspired by patch adding side-by-side diff by Kato Kazuyoshi,
which required $diff_class to be name of class without extra space.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this version of side-by-side diff series.
Junio, as your proposal was of "what if" variety, and not as a proper
patch, I have taken authorship (after reqorking and testing it).
Should I revert authorship to you?
gitweb/gitweb.perl | 67 ++++++++++++++++++++++++++++-----------------------
1 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4f0c3bd..914fd4c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2225,40 +2225,47 @@ sub format_diff_cc_simplified {
return $result;
}
+sub diff_line_class {
+ my ($line, $from, $to) = @_;
+
+ # ordinary diff
+ my $num_sign = 1;
+ # combined diff
+ if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
+ $num_sign = scalar @{$from->{'href'}};
+ }
+
+ my @diff_line_classifier = (
+ { regexp => qr/^\@\@{$num_sign} /, class => "chunk_header"},
+ { regexp => qr/^\\/, class => "incomplete" },
+ { regexp => qr/^ {$num_sign}/, class => "ctx" },
+ # classifier for context must come before classifier add/rem,
+ # or we would have to use more complicated regexp, for example
+ # qr/(?= {0,$m}\+)[+ ]{$num_sign}/, where $m = $num_sign - 1;
+ { regexp => qr/^[+ ]{$num_sign}/, class => "add" },
+ { regexp => qr/^[- ]{$num_sign}/, class => "rem" },
+ );
+ for my $clsfy (@diff_line_classifier) {
+ return $clsfy->{'class'}
+ if ($line =~ $clsfy->{'regexp'});
+ }
+
+ # fallback
+ return "";
+}
+
# format patch (diff) line (not to be used for diff headers)
sub format_diff_line {
my $line = shift;
my ($from, $to) = @_;
- my $diff_class = "";
+
+ my $diff_class = diff_line_class($line, $from, $to);
+ my $diff_classes = "diff";
+ $diff_classes .= " $diff_class" if ($diff_class);
chomp $line;
-
- if ($from && $to && ref($from->{'href'}) eq "ARRAY") {
- # combined diff
- my $prefix = substr($line, 0, scalar @{$from->{'href'}});
- if ($line =~ m/^\@{3}/) {
- $diff_class = " chunk_header";
- } elsif ($line =~ m/^\\/) {
- $diff_class = " incomplete";
- } elsif ($prefix =~ tr/+/+/) {
- $diff_class = " add";
- } elsif ($prefix =~ tr/-/-/) {
- $diff_class = " rem";
- }
- } else {
- # assume ordinary diff
- my $char = substr($line, 0, 1);
- if ($char eq '+') {
- $diff_class = " add";
- } elsif ($char eq '-') {
- $diff_class = " rem";
- } elsif ($char eq '@') {
- $diff_class = " chunk_header";
- } elsif ($char eq "\\") {
- $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}(.*)$/;
@@ -2276,7 +2283,7 @@ sub format_diff_line {
}
$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_classes\">$line</div>\n";
} elsif ($from && $to && $line =~ m/^\@{3}/) {
my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
@@ -2309,9 +2316,9 @@ sub format_diff_line {
}
$line .= " $prefix</span>" .
"<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
- return "<div class=\"diff$diff_class\">$line</div>\n";
+ return "<div class=\"$diff_classes\">$line</div>\n";
}
- return "<div class=\"diff$diff_class\">" . esc_html($line, -nbsp=>1) . "</div>\n";
+ return "<div class=\"$diff_classes\">" . esc_html($line, -nbsp=>1) . "</div>\n";
}
# Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)",
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 2/8] gitweb: Extract formatting of diff chunk header
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 1/8] gitweb: Refactor diff body line classification Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 3/8] gitweb: Add a feature to show side-by-side diff Jakub Narebski
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
Refactor main parts of HTML-formatting for diff chunk headers
(formatting means here adding links and syntax hightlighting) into
separate subroutines:
* format_unidiff_chunk_header for ordinary diff,
* format_cc_diff_chunk_header for combined diff
(more than one parent)
This makes format_diff_line() subroutine easier to follow.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this version of side-by-side diff series.
After those changes format_diff_line() fits in one page (has less than
25 lines). Nice, isn't it?
gitweb/gitweb.perl | 114 ++++++++++++++++++++++++++++++---------------------
1 files changed, 67 insertions(+), 47 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 914fd4c..95d278a 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2254,6 +2254,69 @@ sub diff_line_class {
return "";
}
+# assumes that $from and $to are defined and correctly filled,
+# and that $line holds a line of chunk header for unified diff
+sub format_unidiff_chunk_header {
+ my ($line, $from, $to) = @_;
+
+ my ($from_text, $from_start, $from_lines, $to_text, $to_start, $to_lines, $section) =
+ $line =~ m/^\@{2} (-(\d+)(?:,(\d+))?) (\+(\d+)(?:,(\d+))?) \@{2}(.*)$/;
+
+ $from_lines = 0 unless defined $from_lines;
+ $to_lines = 0 unless defined $to_lines;
+
+ 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 $line;
+}
+
+# assumes that $from and $to are defined and correctly filled,
+# and that $line holds a line of chunk header for combined diff
+sub format_cc_diff_chunk_header {
+ my ($line, $from, $to) = @_;
+
+ my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
+ my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
+
+ @from_text = split(' ', $ranges);
+ for (my $i = 0; $i < @from_text; ++$i) {
+ ($from_start[$i], $from_nlines[$i]) =
+ (split(',', substr($from_text[$i], 1)), 0);
+ }
+
+ $to_text = pop @from_text;
+ $to_start = pop @from_start;
+ $to_nlines = pop @from_nlines;
+
+ $line = "<span class=\"chunk_info\">$prefix ";
+ for (my $i = 0; $i < @from_text; ++$i) {
+ if ($from->{'href'}[$i]) {
+ $line .= $cgi->a({-href=>"$from->{'href'}[$i]#l$from_start[$i]",
+ -class=>"list"}, $from_text[$i]);
+ } else {
+ $line .= $from_text[$i];
+ }
+ $line .= " ";
+ }
+ if ($to->{'href'}) {
+ $line .= $cgi->a({-href=>"$to->{'href'}#l$to_start",
+ -class=>"list"}, $to_text);
+ } else {
+ $line .= $to_text;
+ }
+ $line .= " $prefix</span>" .
+ "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
+ return $line;
+}
+
# format patch (diff) line (not to be used for diff headers)
sub format_diff_line {
my $line = shift;
@@ -2267,56 +2330,13 @@ sub format_diff_line {
$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}(.*)$/;
-
- $from_lines = 0 unless defined $from_lines;
- $to_lines = 0 unless defined $to_lines;
-
- 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>";
+ $line = format_unidiff_chunk_header($line, $from, $to);
return "<div class=\"$diff_classes\">$line</div>\n";
+
} elsif ($from && $to && $line =~ m/^\@{3}/) {
- my ($prefix, $ranges, $section) = $line =~ m/^(\@+) (.*?) \@+(.*)$/;
- my (@from_text, @from_start, @from_nlines, $to_text, $to_start, $to_nlines);
-
- @from_text = split(' ', $ranges);
- for (my $i = 0; $i < @from_text; ++$i) {
- ($from_start[$i], $from_nlines[$i]) =
- (split(',', substr($from_text[$i], 1)), 0);
- }
-
- $to_text = pop @from_text;
- $to_start = pop @from_start;
- $to_nlines = pop @from_nlines;
-
- $line = "<span class=\"chunk_info\">$prefix ";
- for (my $i = 0; $i < @from_text; ++$i) {
- if ($from->{'href'}[$i]) {
- $line .= $cgi->a({-href=>"$from->{'href'}[$i]#l$from_start[$i]",
- -class=>"list"}, $from_text[$i]);
- } else {
- $line .= $from_text[$i];
- }
- $line .= " ";
- }
- if ($to->{'href'}) {
- $line .= $cgi->a({-href=>"$to->{'href'}#l$to_start",
- -class=>"list"}, $to_text);
- } else {
- $line .= $to_text;
- }
- $line .= " $prefix</span>" .
- "<span class=\"section\">" . esc_html($section, -nbsp=>1) . "</span>";
+ $line = format_cc_diff_chunk_header($line, $from, $to);
return "<div class=\"$diff_classes\">$line</div>\n";
+
}
return "<div class=\"$diff_classes\">" . esc_html($line, -nbsp=>1) . "</div>\n";
}
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 3/8] gitweb: Add a feature to show side-by-side diff
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 1/8] gitweb: Refactor diff body line classification Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 2/8] gitweb: Extract formatting of diff chunk header Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 4/8] gitweb: Give side-by-side diff extra CSS styling Jakub Narebski
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
From: Kato Kazuyoshi <kato.kazuyoshi@gmail.com>
This commits adds to support for showing "side-by-side" style diff.
Currently you have to hand-craft the URL; navigation for selecting
diff style is to be added in the next commit.
The diff output in unified format from "git diff-tree" is reorganized to
side-by-side style chunk by chunk with format_sidebyside_diff_chunk().
This reorganization requires knowledge about diff line classification,
so format_diff_line() was renamed to process_diff_line(), and changed to
return tuple (list) consisting of class of diff line and of
HTML-formatted (but not wrapped in <div class="diff ...">...</div>) diff
line. Wrapping is now done by caller, i.e. git_patchset_body().
Gitweb uses float+margin CSS-based layout for "side by side" diff.
You can specify style of diff with "ds" ('diff_style') query
parameter. Currently supported values are 'inline' and 'sidebyside';
the default is 'inline'.
Another solution would be to use "opt" ('extra_options') for that...
though current use of it in gitweb seems to suggest that "opt" is more
about passing extra options to underlying git commands, and "git diff"
doesn't support '--side-by-side' like GNU diff does, (yet?).
Signed-off-by: Kato Kazuyoshi <kato.kazuyoshi@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Heavily changed from original submission by Kato Kazuyoshi, but the
main idea, code structure (somewhat) and CSS-base layout remains.
That's why authorship remains with him; Kato, please speak up if you
would like to change this.
The decision to move some of formatting outside process_diff_line()
(formerly format_diff_line()) was not really necessary, in hindsight...
gitweb/gitweb.perl | 116 +++++++++++++++++++++++++++++++++++++++++----
gitweb/static/gitweb.css | 17 +++++++
2 files changed, 122 insertions(+), 11 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 95d278a..68629f6 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -759,6 +759,7 @@ our @cgi_param_mapping = (
extra_options => "opt",
search_use_regexp => "sr",
ctag => "by_tag",
+ diff_style => "ds",
# this must be last entry (for manipulation from JavaScript)
javascript => "js"
);
@@ -2317,28 +2318,27 @@ sub format_cc_diff_chunk_header {
return $line;
}
-# format patch (diff) line (not to be used for diff headers)
-sub format_diff_line {
+# process patch (diff) line (not to be used for diff headers),
+# returning class and HTML-formatted (but not wrapped) line
+sub process_diff_line {
my $line = shift;
my ($from, $to) = @_;
my $diff_class = diff_line_class($line, $from, $to);
- my $diff_classes = "diff";
- $diff_classes .= " $diff_class" if ($diff_class);
chomp $line;
$line = untabify($line);
if ($from && $to && $line =~ m/^\@{2} /) {
$line = format_unidiff_chunk_header($line, $from, $to);
- return "<div class=\"$diff_classes\">$line</div>\n";
+ return $diff_class, $line;
} elsif ($from && $to && $line =~ m/^\@{3}/) {
$line = format_cc_diff_chunk_header($line, $from, $to);
- return "<div class=\"$diff_classes\">$line</div>\n";
+ return $diff_class, $line;
}
- return "<div class=\"$diff_classes\">" . esc_html($line, -nbsp=>1) . "</div>\n";
+ return $diff_class, esc_html($line, -nbsp=>1);
}
# Generates undef or something like "_snapshot_" or "snapshot (_tbz2_ _zip_)",
@@ -4860,8 +4860,78 @@ sub git_difftree_body {
print "</table>\n";
}
+sub print_sidebyside_diff_chunk {
+ my @chunk = @_;
+ my (@ctx, @rem, @add);
+
+ return unless @chunk;
+
+ # incomplete last line might be among removed or added lines,
+ # or both, or among context lines: find which
+ for (my $i = 1; $i < @chunk; $i++) {
+ if ($chunk[$i][0] eq 'incomplete') {
+ $chunk[$i][0] = $chunk[$i-1][0];
+ }
+ }
+
+ # guardian
+ push @chunk, ["", ""];
+
+ foreach my $line_info (@chunk) {
+ my ($class, $line) = @$line_info;
+
+ # print chunk headers
+ if ($class && $class eq 'chunk_header') {
+ print $line;
+ next;
+ }
+
+ ## print from accumulator when type of class of lines change
+ # empty contents block on start rem/add block, or end of chunk
+ if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) {
+ print join '',
+ '<div class="chunk_block">',
+ '<div class="old">',
+ @ctx,
+ '</div>',
+ '<div class="new">',
+ @ctx,
+ '</div>',
+ '</div>';
+ @ctx = ();
+ }
+ # empty add/rem block on start context block, or end of chunk
+ if ((@rem || @add) && (!$class || $class eq 'ctx')) {
+ print join '',
+ '<div class="chunk_block">',
+ '<div class="old">',
+ @rem,
+ '</div>',
+ '<div class="new">',
+ @add,
+ '</div>',
+ '</div>';
+ @rem = @add = ();
+ }
+
+ ## adding lines to accumulator
+ # guardian value
+ last unless $line;
+ # rem, add or change
+ if ($class eq 'rem') {
+ push @rem, $line;
+ } elsif ($class eq 'add') {
+ push @add, $line;
+ }
+ # context line
+ if ($class eq 'ctx') {
+ push @ctx, $line;
+ }
+ }
+}
+
sub git_patchset_body {
- my ($fd, $difftree, $hash, @hash_parents) = @_;
+ my ($fd, $diff_style, $difftree, $hash, @hash_parents) = @_;
my ($hash_parent) = $hash_parents[0];
my $is_combined = (@hash_parents > 1);
@@ -4871,6 +4941,7 @@ sub git_patchset_body {
my $diffinfo;
my $to_name;
my (%from, %to);
+ my @chunk; # for side-by-side diff
print "<div class=\"patchset\">\n";
@@ -4977,10 +5048,29 @@ sub git_patchset_body {
next PATCH if ($patch_line =~ m/^diff /);
- print format_diff_line($patch_line, \%from, \%to);
+ my ($class, $line) = process_diff_line($patch_line, \%from, \%to);
+ my $diff_classes = "diff";
+ $diff_classes .= " $class" if ($class);
+ $line = "<div class=\"$diff_classes\">$line</div>\n";
+
+ if ($diff_style eq 'sidebyside' && !$is_combined) {
+ if ($class eq 'chunk_header') {
+ print_sidebyside_diff_chunk(@chunk);
+ @chunk = ( [ $class, $line ] );
+ } else {
+ push @chunk, [ $class, $line ];
+ }
+ } else {
+ # default 'inline' style and unknown styles
+ print $line;
+ }
}
} continue {
+ if (@chunk) {
+ print_sidebyside_diff_chunk(@chunk);
+ @chunk = ();
+ }
print "</div>\n"; # class="patch"
}
@@ -6976,6 +7066,7 @@ sub git_object {
sub git_blobdiff {
my $format = shift || 'html';
+ my $diff_style = $input_params{'diff_style'} || 'inline';
my $fd;
my @difftree;
@@ -7085,7 +7176,8 @@ sub git_blobdiff {
if ($format eq 'html') {
print "<div class=\"page_body\">\n";
- git_patchset_body($fd, [ \%diffinfo ], $hash_base, $hash_parent_base);
+ git_patchset_body($fd, $diff_style,
+ [ \%diffinfo ], $hash_base, $hash_parent_base);
close $fd;
print "</div>\n"; # class="page_body"
@@ -7113,6 +7205,7 @@ sub git_blobdiff_plain {
sub git_commitdiff {
my %params = @_;
my $format = $params{-format} || 'html';
+ my $diff_style = $input_params{'diff_style'} || 'inline';
my ($patch_max) = gitweb_get_feature('patches');
if ($format eq 'patch') {
@@ -7316,7 +7409,8 @@ sub git_commitdiff {
$use_parents ? @{$co{'parents'}} : $hash_parent);
print "<br/>\n";
- git_patchset_body($fd, \@difftree, $hash,
+ git_patchset_body($fd, $diff_style,
+ \@difftree, $hash,
$use_parents ? @{$co{'parents'}} : $hash_parent);
close $fd;
print "</div>\n"; # class="page_body"
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 7d88509..21842a6 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -475,6 +475,23 @@ div.diff.nodifferences {
color: #600000;
}
+/* side-by-side diff */
+div.chunk_block {
+ overflow: hidden;
+}
+
+div.chunk_block div.old {
+ float: left;
+ width: 50%;
+ overflow: hidden;
+}
+
+div.chunk_block div.new {
+ margin-left: 50%;
+ width: 50%;
+}
+
+
div.index_include {
border: solid #d9d8d1;
border-width: 0px 0px 1px;
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 4/8] gitweb: Give side-by-side diff extra CSS styling
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
` (2 preceding siblings ...)
2011-10-30 23:36 ` [PATCHv3 3/8] gitweb: Add a feature to show side-by-side diff Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 5/8] t9500: Add test for handling incomplete lines in diff by gitweb Jakub Narebski
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
Use separate background colors for pure removal, pure addition and
change for side-by-side diff. This makes reading such diff easier,
allowing to easily distinguish empty lines in diff from vertical
whitespace used to align chunk blocks.
Note that if lines in diff were numbered, the absence of line numbers
[for one side] would help in distinguishing empty lines from vertical
align.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this version of series.
Note that the code could probably be written with less duplication, at
the cost of being more complicated. I think it is worth tradeoff as
written here.
For how output looks like, compare for example:
http://confluence.atlassian.com/display/FISHEYE/Using+Side+by+Side+Diff+View
gitweb/gitweb.perl | 39 +++++++++++++++++++++++++++++----------
gitweb/static/gitweb.css | 13 +++++++++++++
2 files changed, 42 insertions(+), 10 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 68629f6..f69ed08 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4890,7 +4890,7 @@ sub print_sidebyside_diff_chunk {
# empty contents block on start rem/add block, or end of chunk
if (@ctx && (!$class || $class eq 'rem' || $class eq 'add')) {
print join '',
- '<div class="chunk_block">',
+ '<div class="chunk_block ctx">',
'<div class="old">',
@ctx,
'</div>',
@@ -4902,15 +4902,34 @@ sub print_sidebyside_diff_chunk {
}
# empty add/rem block on start context block, or end of chunk
if ((@rem || @add) && (!$class || $class eq 'ctx')) {
- print join '',
- '<div class="chunk_block">',
- '<div class="old">',
- @rem,
- '</div>',
- '<div class="new">',
- @add,
- '</div>',
- '</div>';
+ if (!@add) {
+ # pure removal
+ print join '',
+ '<div class="chunk_block rem">',
+ '<div class="old">',
+ @rem,
+ '</div>',
+ '</div>';
+ } elsif (!@rem) {
+ # pure addition
+ print join '',
+ '<div class="chunk_block add">',
+ '<div class="new">',
+ @add,
+ '</div>',
+ '</div>';
+ } else {
+ # assume that it is change
+ print join '',
+ '<div class="chunk_block chg">',
+ '<div class="old">',
+ @rem,
+ '</div>',
+ '<div class="new">',
+ @add,
+ '</div>',
+ '</div>';
+ }
@rem = @add = ();
}
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index 21842a6..c7827e8 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -491,6 +491,19 @@ div.chunk_block div.new {
width: 50%;
}
+div.chunk_block.rem div.old div.diff.rem {
+ background-color: #fff5f5;
+}
+div.chunk_block.add div.new div.diff.add {
+ background-color: #f8fff8;
+}
+div.chunk_block.chg div div.diff {
+ background-color: #fffff0;
+}
+div.chunk_block.ctx div div.diff.ctx {
+ color: #404040;
+}
+
div.index_include {
border: solid #d9d8d1;
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 5/8] t9500: Add test for handling incomplete lines in diff by gitweb
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
` (3 preceding siblings ...)
2011-10-30 23:36 ` [PATCHv3 4/8] gitweb: Give side-by-side diff extra CSS styling Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 6/8] t9500: Add basic sanity tests for side-by-side diff in gitweb Jakub Narebski
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
Check that "commitdiff" action in gitweb can handle (without errors)
incomplete lines as added and removed lines, and as context lines.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
New in this series, and independent on side-by-side stuff.
t/t9500-gitweb-standalone-no-errors.sh | 47 ++++++++++++++++++++++++++++++++
1 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 5329715..c731507 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -274,6 +274,53 @@ test_expect_success \
'gitweb_run "p=.git;a=commitdiff;hp=foo-becomes-a-directory;h=foo-symlinked-to-bar"'
# ----------------------------------------------------------------------
+# commitdiff testing (incomplete lines)
+
+test_expect_success 'setup incomplete lines' '
+ cat >file<<-\EOF &&
+ Dominus regit me,
+ et nihil mihi deerit.
+ In loco pascuae ibi me collocavit,
+ super aquam refectionis educavit me;
+ animam meam convertit,
+ deduxit me super semitas jusitiae,
+ propter nomen suum.
+ CHANGE_ME
+ EOF
+ git commit -a -m "Preparing for incomplete lines" &&
+ echo "incomplete" | tr -d "\\012" >>file &&
+ git commit -a -m "Add incomplete line" &&
+ git tag incomplete_lines_add &&
+ sed -e s/CHANGE_ME/change_me/ <file >file+ &&
+ mv -f file+ file &&
+ git commit -a -m "Incomplete context line" &&
+ git tag incomplete_lines_ctx &&
+ echo "Dominus regit me," >file &&
+ echo "incomplete line" | tr -d "\\012" >>file &&
+ git commit -a -m "Change incomplete line" &&
+ git tag incomplete_lines_chg
+ echo "Dominus regit me," >file &&
+ git commit -a -m "Remove incomplete line" &&
+ git tag incomplete_lines_rem
+'
+
+test_expect_success 'commitdiff(1): addition of incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_add"
+'
+
+test_expect_success 'commitdiff(1): incomplete line as context line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_ctx"
+'
+
+test_expect_success 'commitdiff(1): change incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_chg"
+'
+
+test_expect_success 'commitdiff(1): removal of incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_rem"
+'
+
+# ----------------------------------------------------------------------
# commit, commitdiff: merge, large
test_expect_success \
'Create a merge' \
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 6/8] t9500: Add basic sanity tests for side-by-side diff in gitweb
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
` (4 preceding siblings ...)
2011-10-30 23:36 ` [PATCHv3 5/8] t9500: Add test for handling incomplete lines in diff by gitweb Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 7/8] gitweb: Use href(-replay=>1,...) for formats links in "commitdiff" Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 8/8] gitweb: Add navigation to select side-by-side diff Jakub Narebski
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
Test that side-by-side diff can deal with incomplete lines (and while
at it with pure addition, pure removal, and change), and with merge
commits, producing no errors or warnings.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this version.
Note this while it does not check if the output is sane, it can help
with manual check; just run test with `--debug' option, and point
gitweb to "t/trash\ directory.t9500-gitweb-standalone-no-errors/.git/"
repository.
t/t9500-gitweb-standalone-no-errors.sh | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index c731507..ab24917 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -329,7 +329,8 @@ test_expect_success \
git add b &&
git commit -a -m "On branch" &&
git checkout master &&
- git pull . b'
+ git pull . b &&
+ git tag merge_commit'
test_expect_success \
'commit(0): merge commit' \
@@ -379,6 +380,29 @@ test_expect_success \
'gitweb_run "p=.git;a=commitdiff;h=b"'
# ----------------------------------------------------------------------
+# side-by-side diff
+
+test_expect_success 'side-by-side: addition of incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_add;ds=sidebyside"
+'
+
+test_expect_success 'side-by-side: incomplete line as context line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_ctx;ds=sidebyside"
+'
+
+test_expect_success 'side-by-side: changed incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_chg;ds=sidebyside"
+'
+
+test_expect_success 'side-by-side: removal of incomplete line' '
+ gitweb_run "p=.git;a=commitdiff;h=incomplete_lines_rem;ds=sidebyside"
+'
+
+test_expect_success 'side-by-side: merge commit' '
+ gitweb_run "p=.git;a=commitdiff;h=merge_commit;ds=sidebyside"
+'
+
+# ----------------------------------------------------------------------
# tags testing
test_expect_success \
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 7/8] gitweb: Use href(-replay=>1,...) for formats links in "commitdiff"
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
` (5 preceding siblings ...)
2011-10-30 23:36 ` [PATCHv3 6/8] t9500: Add basic sanity tests for side-by-side diff in gitweb Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
2011-10-30 23:36 ` [PATCHv3 8/8] gitweb: Add navigation to select side-by-side diff Jakub Narebski
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
Use href(-replay->1,...) in (sub)navigation links (like changing style
of view, or going to parent commit) so that extra options are
preserved.
This is needed so clicking on such (sub)navigation link would preserve
style of diff; for example when using "side-by-side" diff style then
going to parent commit would now also use this style.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new in this series, and independent on side-by-side stuff.
It can (and perhaps should) be moved earlier in this series.
gitweb/gitweb.perl | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f69ed08..ffaea45 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7267,8 +7267,8 @@ sub git_commitdiff {
}
}
$formats_nav .= ': ' .
- $cgi->a({-href => href(action=>"commitdiff",
- hash=>$hash_parent)},
+ $cgi->a({-href => href(-replay=>1,
+ hash=>$hash_parent, hash_base=>undef)},
esc_html($hash_parent_short)) .
')';
} elsif (!$co{'parent'}) {
@@ -7278,28 +7278,28 @@ sub git_commitdiff {
# single parent commit
$formats_nav .=
' (parent: ' .
- $cgi->a({-href => href(action=>"commitdiff",
- hash=>$co{'parent'})},
+ $cgi->a({-href => href(-replay=>1,
+ hash=>$co{'parent'}, hash_base=>undef)},
esc_html(substr($co{'parent'}, 0, 7))) .
')';
} else {
# merge commit
if ($hash_parent eq '--cc') {
$formats_nav .= ' | ' .
- $cgi->a({-href => href(action=>"commitdiff",
+ $cgi->a({-href => href(-replay=>1,
hash=>$hash, hash_parent=>'-c')},
'combined');
} else { # $hash_parent eq '-c'
$formats_nav .= ' | ' .
- $cgi->a({-href => href(action=>"commitdiff",
+ $cgi->a({-href => href(-replay=>1,
hash=>$hash, hash_parent=>'--cc')},
'compact');
}
$formats_nav .=
' (merge: ' .
join(' ', map {
- $cgi->a({-href => href(action=>"commitdiff",
- hash=>$_)},
+ $cgi->a({-href => href(-replay=>1,
+ hash=>$_, hash_base=>undef)},
esc_html(substr($_, 0, 7)));
} @{$co{'parents'}} ) .
')';
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCHv3 8/8] gitweb: Add navigation to select side-by-side diff
2011-10-30 23:36 [PATCHv3 0/8] gitweb: side-by-side diff Jakub Narebski
` (6 preceding siblings ...)
2011-10-30 23:36 ` [PATCHv3 7/8] gitweb: Use href(-replay=>1,...) for formats links in "commitdiff" Jakub Narebski
@ 2011-10-30 23:36 ` Jakub Narebski
7 siblings, 0 replies; 9+ messages in thread
From: Jakub Narebski @ 2011-10-30 23:36 UTC (permalink / raw)
To: git; +Cc: Kato Kazuyoshi, Jakub Narebski
From: Kato Kazuyoshi <kato.kazuyoshi@gmail.com>
Add to the lower part of navigation bar (the action specific part)
links allowing to switch between 'inline' (ordinary) diff and
'side by side' style diff.
It is not shown for combined / compact combined diff.
Signed-off-by: Kato Kazuyoshi <kato.kazuyoshi@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch was originally part of other commit, but was split into a
separate patch.
Note that "blobdiff" should probably also get this navigation.
gitweb/gitweb.perl | 23 +++++++++++++++++++++++
1 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ffaea45..f80f259 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7164,6 +7164,7 @@ sub git_blobdiff {
my $formats_nav =
$cgi->a({-href => href(action=>"blobdiff_plain", -replay=>1)},
"raw");
+ $formats_nav .= diff_style_nav($diff_style);
git_header_html(undef, $expires);
if (defined $hash_base && (my %co = parse_commit($hash_base))) {
git_print_page_nav('','', $hash_base,$co{'tree'},$hash_base, $formats_nav);
@@ -7221,6 +7222,27 @@ sub git_blobdiff_plain {
git_blobdiff('plain');
}
+# assumes that it is added as later part of already existing navigation,
+# so it returns "| foo | bar" rather than just "foo | bar"
+sub diff_style_nav {
+ my ($diff_style, $is_combined) = @_;
+ $diff_style ||= 'inline';
+
+ return "" if ($is_combined);
+
+ my @styles = (inline => 'inline', 'sidebyside' => 'side by side');
+ my %styles = @styles;
+ @styles =
+ @styles[ map { $_ * 2 } 0..$#styles/2 ];
+
+ return join '',
+ map { " | ".$_ }
+ map {
+ $_ eq $diff_style ? $styles{$_} :
+ $cgi->a({-href => href(-replay=>1, diff_style => $_)}, $styles{$_})
+ } @styles;
+}
+
sub git_commitdiff {
my %params = @_;
my $format = $params{-format} || 'html';
@@ -7250,6 +7272,7 @@ sub git_commitdiff {
$cgi->a({-href => href(action=>"patch", -replay=>1)},
"patch");
}
+ $formats_nav .= diff_style_nav($diff_style, @{$co{'parents'}} > 1);
if (defined $hash_parent &&
$hash_parent ne '-c' && $hash_parent ne '--cc') {
--
1.7.6
^ permalink raw reply related [flat|nested] 9+ messages in thread