From: broquaint Date: Wed, 28 Oct 2009 17:30:58 +0000 (+0000) Subject: Sorted out diffs so that diffing against parents, diffing merges and other such vecto... X-Git-Tag: 0.000000_01~108^2~2 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=ad8884fc358e6a8a37c03589818fcd2b81d68420;p=catagits%2FGitalist.git Sorted out diffs so that diffing against parents, diffing merges and other such vectors work. 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. --- diff --git a/lib/Gitalist/Controller/Root.pm b/lib/Gitalist/Controller/Root.pm index ef231b3..822f3b5 100644 --- a/lib/Gitalist/Controller/Root.pm +++ b/lib/Gitalist/Controller/Root.pm @@ -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', ); diff --git a/lib/Gitalist/Model/Git.pm b/lib/Gitalist/Model/Git.pm index 54d0fd7..8c6f23b 100644 --- a/lib/Gitalist/Model/Git.pm +++ b/lib/Gitalist/Model/Git.pm @@ -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 commit object return a list of hashes corresponding +to the C 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 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 ) 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 commit object return a list of hashes corresponding -to the C 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; diff --git a/lib/Gitalist/View/SyntaxHighlight.pm b/lib/Gitalist/View/SyntaxHighlight.pm index af4e952..688be51 100644 --- a/lib/Gitalist/View/SyntaxHighlight.pm +++ b/lib/Gitalist/View/SyntaxHighlight.pm @@ -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 index 0000000..d91c406 --- /dev/null +++ b/templates/_log_pager.tt2 @@ -0,0 +1,9 @@ +
+ HEAD + [% IF log_lines.size == 50 %] + next + [% END %] + [% IF log_lines.first.sha1 != HEAD %] + prev + [% END %] +
diff --git a/templates/_shortlog.tt2 b/templates/_shortlog.tt2 index 26b6ad7..5d96591 100644 --- a/templates/_shortlog.tt2 +++ b/templates/_shortlog.tt2 @@ -1,15 +1,4 @@ -[% BLOCK pager %] -
- [% IF log_lines.size == 50 %] - next - [% END %] - [% IF log_lines.first.sha1 != HEAD %] - prev - [% END %] -
-[% END %] - -[% INCLUDE pager %] +[% INCLUDE '_log_pager.tt2' %] @@ -56,4 +45,4 @@
-[% INCLUDE pager %] +[% INCLUDE '_log_pager.tt2' %] diff --git a/templates/commit.tt2 b/templates/commit.tt2 index 115e172..97294f0 100644 --- a/templates/commit.tt2 +++ b/templates/commit.tt2 @@ -3,7 +3,7 @@
[% commit.comment.substr(0, 85) %] ... [% FOREACH ref IN branches_on %] - [% ref %] + [% ref %] [% END %]
@@ -17,16 +17,26 @@
commit
[% commit.sha1 %]
tree
-
[% commit.tree_sha1 %] tree
+
[% commit.tree_sha1 %] tree
[% FOREACH parent IN commit.parents %]
parent
-
[% parent %] commit
+
[% parent %] + + commit + diff + +
[% END %] 
[% commit.comment %]
-[% 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; +%]