Sorted out diffs so that diffing against parents, diffing merges and other such vecto...
broquaint [Wed, 28 Oct 2009 17:30:58 +0000 (17:30 +0000)]
Also tidied up the paging and added it to the /log action.
For the record the "solution" for sorting out the diffs is not ideal and could do with refactoring.

lib/Gitalist/Controller/Root.pm
lib/Gitalist/Model/Git.pm
lib/Gitalist/View/SyntaxHighlight.pm
templates/_log_pager.tt2 [new file with mode: 0644]
templates/_shortlog.tt2
templates/commit.tt2
templates/commitdiff.tt2
templates/log.tt2

index ef231b3..822f3b5 100644 (file)
@@ -184,14 +184,17 @@ sub blobdiff : Local {
   my $commit = $self->_get_commit($c);
   my $filename = $c->req->param('f')
               || croak("No file specified!");
-  my @diff = $c->model('Git')->diff(
-    $commit->parent_sha1, $commit->sha1, '--', $filename
+  my($tree, $patch) = $c->model('Git')->diff(
+    commit => $commit,
+    parent => $c->req->param('hp') || '',
+    file   => $filename,
+    patch  => 1,
   );
   $c->stash(
     commit    => $commit,
-    diff      => \@diff,
+    diff      => $patch,
     # XXX Hack hack hack, see View::SyntaxHighlight
-    blobs     => [$diff[0]->{diff}],
+    blobs     => [$patch->[0]->{diff}],
     language  => 'Diff',
     action    => 'blobdiff',
   );
@@ -211,7 +214,7 @@ sub commit : Local {
   my $commit = $self->_get_commit($c);
   $c->stash(
       commit      => $commit,
-      diff_tree   => [$c->model('Git')->diff_tree($commit)],
+      diff_tree   => ($c->model('Git')->diff(commit => $commit))[0],
       branches_on => [$c->model('Git')->refs_for($commit->sha1)],
       action      => 'commit',
   );
@@ -227,13 +230,17 @@ sub commitdiff : Local {
   my ( $self, $c ) = @_;
 
   my $commit = $self->_get_commit($c);
-  my @difflist = $c->model('Git')->diff($commit->parent_sha1, $commit->sha1);
+  my($tree, $patch) = $c->model('Git')->diff(
+      commit => $commit,
+      parent => $c->req->param('hp') || '',
+      patch  => 1,
+  );
   $c->stash(
     commit    => $commit,
-    diff_tree => [$c->model('Git')->diff_tree($commit)],
-    diff      => \@difflist,
+    diff_tree => $tree,
+    diff      => $patch,
     # XXX Hack hack hack, see View::SyntaxHighlight
-    blobs     => [map $_->{diff}, @difflist],
+    blobs     => [map $_->{diff}, @$patch],
     language  => 'Diff',
     action    => 'commitdiff',
   );
index 54d0fd7..8c6f23b 100644 (file)
@@ -13,7 +13,7 @@ use Carp qw/croak/;
 use File::Find::Rule;
 use DateTime::Format::Mail;
 use File::Stat::ModeString;
-use List::MoreUtils qw/any/;
+use List::MoreUtils qw/any zip/;
 use Scalar::Util qw/blessed/;
 use MooseX::Types::Common::String qw/NonEmptySimpleStr/; # FIXME, use Types::Path::Class and coerce
 
@@ -417,13 +417,21 @@ Provides the raw output of a diff.
 
 =cut
 
+# gitweb uses the following sort of command for diffing merges:
+# /home/dbrook/apps/bin/git --git-dir=/home/dbrook/dev/app/.git diff-tree -r -M --no-commit-id --patch-with-raw --full-index --cc 316cf158df3f6207afbae7270bcc5ba0 --
+# and for regular diffs
+# /home/dbrook/apps/bin/git --git-dir=/home/dbrook/dev/app/.git diff-tree -r -M --no-commit-id --patch-with-raw --full-index 2e3454ca0749641b42f063730b0090e1 316cf158df3f6207afbae7270bcc5ba0 --
+
 sub raw_diff {
   my ($self, @args) = @_;
 
-  return $self->command(diff => '--full-index', @args);
+  return $self->command(
+         qw(diff-tree -r -M --no-commit-id --full-index),
+         @args
+  );
 }
 
-=begin
+=pod
 diff --git a/TODO b/TODO
 index 6a05e77..2071fd0 100644
 --- a/TODO
@@ -453,10 +461,37 @@ and some associated metadata.
 
 =cut
 
+# XXX Ideally this would return a wee object instead of ad hoc structures.
 sub diff {
-  my($self, @revs) = @_;
+  my($self, %args) = @_;
+
+  # So either a parent is specifed, or we use the commit's parent if there's
+  # only one, otherwise it was a merge commit.
+  my $parent = $args{parent}
+                        ? $args{parent}
+                        : @{$args{commit}->parents} <= 1
+                          ? $args{commit}->parent_sha1
+                          : '-c';
+  my @etc = (
+    ( $args{file}  ? ('--', $args{file}) : () ),
+  );
+
+  my @out = $self->raw_diff(
+       ( $args{patch} ? '--patch-with-raw' : () ),
+         $parent, $args{commit}->sha1, @etc
+  );
 
-  return $self->parse_diff($self->raw_diff(@revs));
+  # XXX Yes, there is much wrongness having parse_diff_tree be destructive.
+  my @difftree = $self->parse_diff_tree(\@out);
+
+  return \@difftree
+       unless $args{patch};
+
+  # The blank line between the tree and the patch.
+  shift @out;
+
+  # XXX And no I'm not happy about having diff return tree + patch.
+  return \@difftree, [$self->parse_diff(@out)];
 }
 
 sub parse_diff {
@@ -488,6 +523,37 @@ sub parse_diff {
   return @ret;
 }
 
+# $ git diff-tree -r --no-commit-id -M b222ff0a7260cc1777c7e455dfcaf22551a512fc 7e54e579e196c6c545fee1030175f65a111039d4
+# :100644 100644 6a85d6c6315b55a99071974eb6ce643aeb2799d6 44c03ed6c328fa6de4b1d9b3f19a3de96b250370 M      templates/blob.tt2
+
+=head2 parse_diff_tree
+
+Given a L<Git::PurePerl> commit object return a list of hashes corresponding
+to the C<diff-tree> output.
+
+=cut
+
+sub parse_diff_tree {
+  my($self, $diff) = @_;
+
+  my @keys = qw(modesrc modedst sha1src sha1dst status src dst);
+  my @ret;
+  while($diff->[0] =~ /^:\d+/) {
+       local $_ = shift @$diff;
+    # see. man git-diff-tree for more info
+    # mode src, mode dst, sha1 src, sha1 dst, status, src[, dst]
+    my @vals = /^:(\d+) (\d+) ($SHA1RE) ($SHA1RE) ([ACDMRTUX])\t([^\t]+)(?:\t([^\n]+))?$/;
+    my %line = zip @keys, @vals;
+    # Some convenience keys
+    $line{file}   = $line{src};
+    $line{sha1}   = $line{sha1dst};
+    $line{is_new} = $line{sha1src} =~ /^0+$/;
+    push @ret, \%line;
+  }
+
+  return @ret;
+}
+
 =head2 parse_rev_list
 
 Given the output of the C<rev-list> command return a list of hashes.
@@ -571,8 +637,7 @@ sub reflog {
     =  $self->run_cmd_in($self->project, qw(log -g), @logargs)
     =~ /(^commit.+?(?:(?=^commit)|(?=\z)))/msg;
 
-=begin
-
+=pod
   commit 02526fc15beddf2c64798a947fecdd8d11bf993d
   Reflog: HEAD@{14} (The Git Server <git@git.dev.venda.com>)
   Reflog message: push
@@ -683,51 +748,6 @@ sub references {
        return $self->{references} = \%refs;
 }
 
-=begin
-
-$ git diff-tree -r --no-commit-id -M b222ff0a7260cc1777c7e455dfcaf22551a512fc 7e54e579e196c6c545fee1030175f65a111039d4
-:100644 100644 8976ebc7df65475b3def53a1653533c3f61070d0 852b6e170f1bad1fbd9930d3178dda8fdf1feae7 M      TODO
-:100644 100644 75f5e5f9ed10ae82a960fde77ecf138159c37610 7f54f8c3a4ad426f6889b13cfba5f5ad9969e3c6 M      lib/Gitalist/Controller/Root.pm
-:100644 100644 2c65caa46b56302502b9e6eef952b6f379c71fee e418acf5f7b5f771b0b2ef8be784e8dcd60a4271 M      lib/Gitalist/View/Default.pm
-:000000 100644 0000000000000000000000000000000000000000 642599f9ccfc4dbc7034987ad3233655010ff348 A      lib/Gitalist/View/SyntaxHighlight.pm
-:000000 100644 0000000000000000000000000000000000000000 3d2e533c41f01276b6f844bae98297273b38dffc A      root/static/css/syntax-dark.css
-:100644 100644 6a85d6c6315b55a99071974eb6ce643aeb2799d6 44c03ed6c328fa6de4b1d9b3f19a3de96b250370 M      templates/blob.tt2
-
-=cut
-
-use List::MoreUtils qw(zip);
-# XXX Hrm, getting called twice, not sure why.
-=head2 diff_tree
-
-Given a L<Git::PurePerl> commit object return a list of hashes corresponding
-to the C<diff-tree> output.
-
-=cut
-
-sub diff_tree {
-       my($self, $commit) = @_;
-
-       my @dtout = $self->command(
-               # XXX should really deal with multple parents ...
-               qw(diff-tree -r --no-commit-id -M), $commit->parent_sha1, $commit->sha1
-       );
-
-       my @keys = qw(modesrc modedst sha1src sha1dst status src dst);
-       my @difftree = map {
-               # see. man git-diff-tree for more info
-               # mode src, mode dst, sha1 src, sha1 dst, status, src[, dst]
-               my @vals = /^:(\d+) (\d+) ($SHA1RE) ($SHA1RE) ([ACDMRTUX])\t([^\t]+)(?:\t([^\n]+))?$/;
-               my %line = zip @keys, @vals;
-               # Some convenience keys
-               $line{file}   = $line{src};
-               $line{sha1}   = $line{sha1dst};
-               $line{is_new} = $line{sha1src} =~ /^0+$/;
-               \%line;
-       } @dtout;
-
-       return @difftree;
-}
-
 1;
 
 __PACKAGE__->meta->make_immutable;
index af4e952..688be51 100644 (file)
@@ -44,6 +44,7 @@ sub process {
     $c->forward('View::Default');
 }
 
+# XXX This takes for freakin' ever on big merges. A cache may be needed.
 sub highlight {
     my($self, $lang, $blob) = @_;
 
diff --git a/templates/_log_pager.tt2 b/templates/_log_pager.tt2
new file mode 100644 (file)
index 0000000..d91c406
--- /dev/null
@@ -0,0 +1,9 @@
+<div class='pager'>
+ <a href='/[% action %]?p=[% project %];h=[% HEAD %]'>HEAD</a>
+ [% IF log_lines.size == 50 %]
+ <a href='/[% action %]?p=[% project %];h=[% commit.sha1 %];pg=[% page %]'>next</a>
+ [% END %]
+ [% IF log_lines.first.sha1 != HEAD %]
+ <a href='/[% action %]?p=[% project %];h=[% commit.sha1 %];pg=[% page - 1 %]'>prev</a>
+ [% END %]
+</div>
index 26b6ad7..5d96591 100644 (file)
@@ -1,15 +1,4 @@
-[% BLOCK pager %]
-<div class='pager'>
- [% IF log_lines.size == 50 %]
- <a href='/[% action %]?p=[% project %];h=[% commit.sha1 %];pg=[% page %]'>next</a>
- [% END %]
- [% IF log_lines.first.sha1 != HEAD %]
- <a href='/[% action %]?p=[% project %];h=[% commit.sha1 %];pg=[% page - 1 %]'>prev</a>
- [% END %]
-</div>
-[% END %]
-
-[% INCLUDE pager %]
+[% INCLUDE '_log_pager.tt2' %]
 <table>
  <thead>
   <tr>
@@ -56,4 +45,4 @@
   </tr>
  </tfoot>
 </table>
-[% INCLUDE pager %]
+[% INCLUDE '_log_pager.tt2' %]
index 115e172..97294f0 100644 (file)
@@ -3,7 +3,7 @@
 <div class='commit-message'>
 [% commit.comment.substr(0, 85) %] ...
 [% FOREACH ref IN branches_on %]
- <span class='refs'><a href='/shortlog?h=[% commit.sha1 %];hb=[% ref %]'>[% ref %]</a></span>
+ <span class='refs'><a href='/shortlog?p=[% project %];h=[% commit.sha1 %];hb=[% ref %]'>[% ref %]</a></span>
 [% END %]
 </div>
 
  <dt>commit</dt>
   <dd>[% commit.sha1 %]</dd>
  <dt>tree</dt>
-  <dd>[% commit.tree_sha1 %] <a href="/tree?h=[% commit.sha1 %];hb=[% commit.tree_sha1 %]">tree</a></dd>
+  <dd>[% commit.tree_sha1 %] <a href="/tree?p=[% project %];h=[% commit.sha1 %];hb=[% commit.tree_sha1 %]">tree</a></dd>
  [% FOREACH parent IN commit.parents %]
  <dt>parent</dt>
-  <dd>[% parent %] <a href="/commit?h=[% commit.sha1 %]">commit</a></dd>
+  <dd>[% parent %]
+    <span class='action-list'>
+        <a href="/commit?p=[% project %];h=[% parent %]">commit</a>
+        <a href="/commitdiff?p=[% project %];h=[% commit.sha1 %];hp=[% parent %]">diff</a>
+       </span>
+   </dd>
  [% END %]
 </dl>
 
 <pre class='commit-message'>[% commit.comment %]</pre>
 
-[% INCLUDE '_diff_tree.tt2' %]
+[%
+# In the case of merge commits there will be no diff tree.
+IF diff_tree.size > 0;
+       INCLUDE '_diff_tree.tt2';
+END;
+%]
 
 <!--
 
index acb61c4..a106b26 100644 (file)
@@ -8,6 +8,23 @@
   [% commit.author.name | html %] [[% commit.authored_time %]]
 </div>
 
-[% INCLUDE '_diff_tree.tt2' %]
 
-[% INCLUDE '_diff.tt2' %]
+[%
+# In the case of merge commits there will be no diff tree.
+IF diff_tree.size > 0;
+  INCLUDE '_diff_tree.tt2';
+END;
+IF diff.size > 0;
+  INCLUDE '_diff.tt2';
+ELSE
+%]
+<div class='no-difference'>
+[%
+  IF commit.parents > 1;
+    'Trivial merge';
+  ELSE;
+    'No differences found';
+  END;
+%]
+</div>
+[% END %]
index c8c3c21..2953d4e 100644 (file)
@@ -1,5 +1,7 @@
 [% INCLUDE 'commit-nav.tt2' object = commit %]
 
+[% INCLUDE '_log_pager.tt2' %]
+
 [%# XXX Nabbed the HTML below from gitweb's log action. %]
 [% FOREACH line IN log_lines %]
 <div class="header">
@@ -22,3 +24,5 @@
  [% line.comment | html %]
 </div>
 [% END %]
+
+[% INCLUDE '_log_pager.tt2' %]