repobrowse: linkify hunk headers

A work-in-progress, but this provides useful links to the
exact line number of modified lines instead of blindly linking
to the top-of-the-file.  Avoid flooding the user with links
but give hints by bolding the final resulting file or SHA-1
object ID.

Do other repository viewers do this?

diff --git a/lib/PublicInbox/Hval.pm b/lib/PublicInbox/Hval.pm
index 2e8b1d60c5054b3e5d18670f9b97518072fb46f4..a16cd4a1a8d1a69ea11e2ac84ebefb99be438134 100644 (file)
--- a/lib/PublicInbox/Hval.pm
+++ b/lib/PublicInbox/Hval.pm
@@ -74,6 +74,12 @@ sub ascii_html {
 sub as_html { ascii_html($_[0]->{raw}) }
 sub as_href { ascii_html(uri_escape_utf8($_[0]->{href})) }
 
+sub as_path {
+       my $p = uri_escape_utf8($_[0]->{href});
+       $p =~ s!%2[fF]!/!g;
+       ascii_html($p);
+}
+
 sub raw {
        if (defined $_[1]) {
                $_[0]->{raw} = $_[1];

diff --git a/lib/PublicInbox/RepoBrowseGitCommit.pm b/lib/PublicInbox/RepoBrowseGitCommit.pm
index 324c60a1f27e6bf5917cd026f78d3a9176fab765..2814d53f93da55cd244c3a121837850e057dce6e 100644 (file)
--- a/lib/PublicInbox/RepoBrowseGitCommit.pm
+++ b/lib/PublicInbox/RepoBrowseGitCommit.pm
@@ -6,13 +6,17 @@ use strict;
 use warnings;
 use base qw(PublicInbox::RepoBrowseBase);
 use PublicInbox::Git;
+use PublicInbox::RepoBrowseGit qw(git_unquote);
 
 use constant GIT_FMT => '--pretty=format:'.join('%n',
-       '%H', '%s', '%an <%ae>', '%ai', '%cn <%ce>', '%ci',
+       '%H', '%h', '%s', '%an <%ae>', '%ai', '%cn <%ce>', '%ci',
        '%t', '%p', '%D', '%b%x00');
 
+*ascii_html = *PublicInbox::Hval::ascii_html;
+
 sub git_commit_stream {
        my ($req, $q, $H, $log, $fh) = @_;
+       chomp(my $h = <$log>); # abbreviated commit
        my $l;
        my $s = PublicInbox::Hval->new_oneline($l = <$log>)->as_html; # subject
        my $au = PublicInbox::Hval->new_oneline($l = <$log>)->as_html; # author
@@ -35,47 +39,45 @@ sub git_commit_stream {
                $x .= '   parent ';
                $x .= qq(<a\nhref="${rel}commit?id=$p[0]">$p[0]</a>\n);
        } elsif (scalar(@p) > 1) {
-               $x .= '    merge ';
-               $x .= join(' ', map {
-                       qq(<a\nhref="${rel}commit?id=$_">$_</a>)
-                       } @p);
-               $x .= "\n";
+               foreach my $p (@p) {
+                       $x .= '    merge ';
+                       $x .= "<a\nhref=\"${rel}commit?id=$p\">$p</a> (";
+                       $x .= "<a\nhref=\"${rel}diff?id=$p&id2=$h\">";
+                       $x .= "diff</a>)\n";
+               }
        }
        $fh->write($x .= "\n$s\n\n");
 
        # body:
        local $/ = "\0";
-       $x = PublicInbox::Hval->new($l = <$log>)->as_html; # body
-       $fh->write($x);
+       $l = <$log>;
+       chomp $l;
+       $x = PublicInbox::Hval->new_bin($l)->as_html; # body
+       $fh->write($x."\n");
+
+       git_show_diffstat($fh, $log);
 
        # diff
        local $/ = "\n";
        my $cmt = '[a-f0-9]+';
-       my @href;
+       my $diff = { h => $h, p => \@p, rel => $rel };
        while (defined($l = <$log>)) {
-               if ($l =~ /^index ($cmt)\.\.($cmt)(.*)$/o) { # regular
-                       my $end = $3;
-                       my @l = ($1, $2);
-                       @href = git_blob_hrefs($rel, @l);
-                       @l = git_blob_links(\@href, \@l);
-                       $l = "index $l[0]..$l[1]$end\n";
+               if ($l =~ m{^diff --git ("?a/.+) ("?b/.+)$}) { # regular
+                       $l = git_diff_ab_hdr($diff, $1, $2) . "\n";
+               } elsif ($l =~ m{^diff --cc (.+)$}) { # --cc
+                       $l = git_diff_cc_hdr($diff, $1) . "\n";
+               } elsif ($l =~ /^index ($cmt)\.\.($cmt)(.*)$/o) { # regular
+                       $l = git_diff_ab_index($diff, $1, $2, $3) . "\n";
                } elsif ($l =~ /^@@ (\S+) (\S+) @@(.*)$/) { # regular
-                       my $ctx = $3;
-                       my @l = ($1, $2);
-                       @l = git_blob_links(\@href, \@l);
-                       $l = "@@ $l[0] $l[1] \@\@$ctx\n";
+                       $l = git_diff_ab_hunk($diff, $1, $2, $3) . "\n";
+               } elsif ($l =~ /^\+/) { # added hunk
+                       chomp $l;
+                       $l = '<b>'.PublicInbox::Hval->new_bin($l)->as_html.
+                               "</b>\n";
                } elsif ($l =~ /^index ($cmt,[^\.]+)\.\.($cmt)(.*)$/o) { # --cc
-                       my @l = (split(',', $1), $2);
-                       my $end = $3;
-                       @href = git_blob_hrefs($rel, @l);
-                       @l = git_blob_links(\@href, \@l);
-                       my $res = pop @l;
-                       $l = 'index '.join(',', @l)."..$res$end\n";
+                       $l = git_diff_cc_index($diff, $1, $2, $3) . "\n";
                } elsif ($l =~ /^(@@@+) (\S+.*\S+) @@@+(.*)$/) { # --cc
-                       my ($at, $ctx) = ($1, $3);
-                       my @l = split(' ', $2);
-                       @l = git_blob_links(\@href, \@l);
-                       $l = join(' ', $at, @l, $at) . $ctx . "\n";
+                       $l = git_diff_cc_hunk($diff, $1, $2, $3) . "\n";
                } else {
                        $l = PublicInbox::Hval->new_bin($l)->as_html;
                }
@@ -93,8 +95,8 @@ sub call_git_commit {
        my $id = $q->{id};
        $id eq '' and $id = 'HEAD';
        my $git = $repo_info->{git} ||= PublicInbox::Git->new($path);
-       my $log = $git->popen(qw(show --no-notes --no-color
-                                --abbrev=16 --irreversible-delete),
+       my $log = $git->popen(qw(show -z --numstat -p --no-notes --no-color
+                                --abbrev=16),
                                 GIT_FMT, $id);
        my $H = <$log>;
        defined $H or return;
@@ -117,4 +119,130 @@ sub git_blob_links {
        map { $hrefs->[$i++].">$_</a>" } @$labels;
 }
 
+sub git_show_diffstat {
+       my ($fh, $log) = @_;
+       local $/ = "\0\0";
+       my $l = <$log>;
+       chomp $l;
+       my @stat = split("\0", $l);
+       my $nr = 0;
+       my ($nadd, $ndel) = (0, 0);
+       while (defined($l = shift @stat)) {
+               $l =~ s/\n?(\S+)\t+(\S+)\t+// or next;
+               my ($add, $del) = ($1, $2);
+               if ($add =~ /\A\d+\z/) {
+                       $nadd += $add;
+                       $ndel += $del;
+                       $add = "+$add";
+                       $del = "-$del";
+               }
+               my $num = sprintf('% 6s/%-6s', $del, $add);
+               unless (length $l) {
+                       my $from = shift @stat;
+                       my $to = shift @stat;
+                       $l = "$from => $to";
+               }
+               $l = PublicInbox::Hval->new_bin($l)->as_html;
+               ++$nr;
+               $fh->write($num."\t".$l."\n");
+       }
+       $l = "\n$nr ";
+       $l .= $nr == 1 ? 'file changed, ' : 'files changed, ';
+       $l .= $nadd;
+       $l .= $nadd == 1 ? ' insertion(+), ' : ' insertions(+), ';
+       $l .= $ndel;
+       $l .= $ndel == 1 ? " deletion(-)\n\n" : " deletions(-)\n\n";
+       $fh->write($l);
+}
+
+# index abcdef89..01234567
+sub git_diff_ab_index {
+       my ($diff, $xa, $xb, $end) = @_;
+       # not wasting bandwidth on links here, yet
+       # links in hunk headers are far more useful with line offsets
+       $end = PublicInbox::Hval->new_bin($end)->as_html;
+       "index $xa..<b>$xb</b>$end";
+}
+
+# diff --git a/foo.c b/bar.c
+sub git_diff_ab_hdr {
+       my ($diff, $fa, $fb) = @_;
+       my $html_a = PublicInbox::Hval->new_bin($fa)->as_html;
+       my $html_b = PublicInbox::Hval->new_bin($fb)->as_html;
+       $fa = git_unquote($fa);
+       $fb = git_unquote($fb);
+       $fa =~ s!\Aa/!!;
+       $fb =~ s!\Ab/!!;
+       $fa = $diff->{fa} = PublicInbox::Hval->new_bin($fa);
+       $fb = $diff->{fb} = PublicInbox::Hval->new_bin($fb);
+       $diff->{path_a} = $fa->as_path;
+       $diff->{path_b} = $fb->as_path;
+
+       # not wasting bandwidth on links here, yet
+       # links in hunk headers are far more useful with line offsets
+       "diff --git $html_a <b>$html_b</b>";
+}
+
+# @@ -1,2 +3,4 @@ (regular diff)
+sub git_diff_ab_hunk {
+       my ($diff, $ca, $cb, $ctx) = @_;
+       my ($na) = ($ca =~ /\A-(\d+),/);
+       my ($nb) = ($cb =~ /\A\+(\d+),/);
+
+       my $rel = $diff->{rel};
+       my $p = $diff->{p}->[0];
+       my $h = $diff->{h};
+
+       '@@ ' .
+       "<a\nhref=\"${rel}tree/$diff->{path_a}?id=$p#n$na\">$ca</a> " .
+       "<a\nhref=\"${rel}tree/$diff->{path_b}?id=$h#n$nb\"><b>$cb</b></a> " .
+       '@@' . PublicInbox::Hval->new_bin($ctx)->as_html;
+}
+
+sub git_diff_cc_hdr {
+       my ($diff, $path) = @_;
+       my $html_path = PublicInbox::Hval->new_bin($path)->as_html;
+       my $cc = $diff->{cc} = PublicInbox::Hval->new_bin(git_unquote($path));
+       $diff->{path_cc} = $cc->as_path;
+       "diff --cc <b>$html_path</b>";
+}
+
+# index abcdef09,01234567..76543210
+sub git_diff_cc_index {
+       my ($diff, $before, $last, $end) = @_;
+       $end = PublicInbox::Hval->new_bin($end)->as_html;
+       $diff->{pobj_cc} = [ split(',', $before) ];
+
+       # not wasting bandwidth on links here, yet
+       # links in hunk headers are far more useful with line offsets
+       "index $before..<b>$last</b>$end";
+}
+
+# @@@ -1,2 -3,4 +5,6 @@@ (combined diff)
+sub git_diff_cc_hunk {
+       my ($diff, $at, $offs, $ctx) = @_;
+       my @offs = split(' ', $offs);
+       my $last = pop @offs;
+       my @p = @{$diff->{p}};
+       my @pobj = @{$diff->{pobj_cc}};
+       my $path = $diff->{path_cc};
+       my $rel = $diff->{rel};
+       my $rv = $at;
+
+       # special 'cc' action as we don't have reliable paths from parents
+       my $ppath = "${rel}cc/$path";
+       foreach my $off (@offs) {
+               my $p = shift @p;
+               my $obj = shift @pobj; # blob SHA-1
+               my ($n) = ($off =~ /\A-(\d+),/); # line number
+               $rv .= " <a\nhref=\"$ppath?id=$p&obj=$obj#n$n\">$off</a>";
+       }
+
+       # we can use the normal 'tree' endpoint for the result
+       my ($n) = ($last =~ /\A\+(\d+),/); # line number
+       my $h = $diff->{h};
+       $rv .= " <a\nhref=\"${rel}tree/$path?id=$h#n$n\"><b>$last</b></a>";
+       $rv .= " $at" . PublicInbox::Hval->new_bin($ctx)->as_html;
+}
+
 1;