Relocate diff method from ::Project to ::Object.
Zachary Stevens [Sat, 14 Nov 2009 00:52:37 +0000 (00:52 +0000)]
For the time being, Project still implements diff, delegating to Object.

lib/Gitalist/Git/Object.pm
lib/Gitalist/Git/Project.pm

index 3addbb6..e42f673 100644 (file)
@@ -2,9 +2,13 @@ use MooseX::Declare;
 use Moose::Autobox;
 
 class Gitalist::Git::Object {
-    use MooseX::Types::Moose qw/Str Int/;
+    use MooseX::Types::Moose qw/Str Int Bool Maybe/;
     use MooseX::Types::Common::String qw/NonEmptySimpleStr/;
     use File::Stat::ModeString qw/mode_to_string/;
+    use List::MoreUtils qw/any zip/;
+
+    our $SHA1RE = qr/[0-9a-fA-F]{40}/;
+
     # project and sha1 are required initargs
     has project => ( isa => 'Gitalist::Git::Project',
                      required => 1,
@@ -12,17 +16,18 @@ class Gitalist::Git::Object {
                      weak_ref => 1,
                      handles => {
                          _run_cmd => 'run_cmd',
+                         _run_cmd_list => 'run_cmd_list',
                          _get_gpp_object => 'get_gpp_object',
                      },
                  );
     has sha1 => ( isa => NonEmptySimpleStr,
-               required => 1,
-               is => 'ro' );
+                  required => 1,
+                  is => 'ro' );
 
     has $_ => ( isa => NonEmptySimpleStr,
-                  required => 1,
-                  is => 'ro',
-                  lazy_build => 1 )
+                required => 1,
+                is => 'ro',
+                lazy_build => 1 )
         for qw/type modestr size/;
 
     has _gpp_obj => ( isa => 'Git::PurePerl::Object',
@@ -57,13 +62,111 @@ class Gitalist::Git::Object {
                   required => 0,
                   is => 'ro' );
     has mode => ( isa => Int,
-                required => 1,
-                default => 0,
-                is => 'ro' );
+                  required => 1,
+                  default => 0,
+                  is => 'ro' );
 
     method BUILD { $self->$_() for qw/_gpp_obj type size modestr/ }
 
-    method _build__gpp_obj {
+
+    method diff ( Maybe[Bool] :$patch?,
+                  Maybe[NonEmptySimpleStr] :$parent?,
+                  Maybe[NonEmptySimpleStr] :$file?
+              ) {
+        # Use parent if specifed, else take the parent from the commit
+        # if there is only one, otherwise it was a merge commit.
+        $parent = $parent
+            ? $parent
+                : $self->parents <= 1
+                    ? $self->parent_sha1
+                        : '-c';
+        my @etc = (
+            ( $file  ? ('--', $file) : () ),
+        );
+
+        my @out = $self->_raw_diff(
+            ( $patch ? '--patch-with-raw' : () ),
+            ( $parent ? $parent : () ),
+            $self->sha1, @etc,
+        );
+
+        # XXX Yes, there is much wrongness having _parse_diff_tree be destructive.
+        my @difftree = $self->_parse_diff_tree(\@out);
+
+        return \@difftree
+            unless $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)];
+    }
+
+## Private methods
+    # 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 --
+    method _raw_diff (@args) {
+        return $self->_run_cmd_list(
+            qw(diff-tree -r -M --no-commit-id --full-index),
+            @args
+        );
+    }
+
+    method _parse_diff_tree ($diff) {
+        my @keys = qw(modesrc modedst sha1src sha1dst status src dst);
+        my @ret;
+        while (@$diff and $diff->[0] =~ /^:\d+/) {
+            my $line = shift @$diff;
+            # see. man git-diff-tree for more info
+            # mode src, mode dst, sha1 src, sha1 dst, status, src[, dst]
+            my @vals = $line =~ /^:(\d+) (\d+) ($SHA1RE) ($SHA1RE) ([ACDMRTUX]\d*)\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+$/
+               if $line{sha1src};
+            @line{qw/status sim/} = $line{status} =~ /(R)(\d+)/
+                if $line{status} =~ /^R/;
+            push @ret, \%line;
+        }
+
+        return @ret;
+    }
+
+    method _parse_diff (@diff) {
+        my @ret;
+        for (@diff) {
+            # This regex is a little pathological.
+            if (m{^diff --git (a/(.*?)) (b/\2)}) {
+                push @ret, {
+                    head => $_,
+                    a    => $1,
+                    b    => $3,
+                    file => $2,
+                    diff => '',
+                };
+                next;
+            }
+
+            if (/^index (\w+)\.\.(\w+) (\d+)$/) {
+                @{$ret[-1]}{qw(index src dst mode)} = ($_, $1, $2, $3);
+                next
+            }
+
+            # XXX Somewhat hacky. Ahem.
+            $ret[@ret ? -1 : 0]{diff} .= "$_\n";
+        }
+
+        return @ret;
+    }
+
+
+## Builders
+method _build__gpp_obj {
         return $self->_get_gpp_object($self->sha1)
     }
 
index 9d90ea4..43a183c 100644 (file)
@@ -247,45 +247,18 @@ Returns a list of revs for the given head ($sha1).
 
 =head2 diff($commit, $patch?, $parent?, $file?)
 
-Generate a diff.
-
-FIXME this should be a method on the commit object.
+Generate a diff from a given L<Gitalist::Git::Object>.
 
 =cut
 
-    # XXX Ideally this would return a wee object instead of ad hoc structures.
     method diff ( Gitalist::Git::Object :$commit!,
                   Bool :$patch?,
                   Maybe[NonEmptySimpleStr] :$parent?,
-                  NonEmptySimpleStr :$file? ) {
-        # Use parent if specifed, else take the parent from the commit
-        # if there is only one, otherwise it was a merge commit.
-        $parent = $parent
-            ? $parent
-            : $commit->parents <= 1
-            ? $commit->parent_sha1
-            : '-c';
-        my @etc = (
-            ( $file  ? ('--', $file) : () ),
-        );
-
-        my @out = $self->_raw_diff(
-            ( $patch ? '--patch-with-raw' : () ),
-            ( $parent ? $parent : () ),
-            $commit->sha1, @etc,
-        );
-
-        # XXX Yes, there is much wrongness having _parse_diff_tree be destructive.
-        my @difftree = $self->_parse_diff_tree(\@out);
-
-        return \@difftree
-            unless $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)];
+                  NonEmptySimpleStr :$file?
+              ) {
+              return $commit->diff( patch => $patch,
+                                    parent => $parent,
+                                    file => $file);
     }
 
 =head2 reflog(@lorgargs)
@@ -413,65 +386,6 @@ FIXME Should this return objects?
                     map  split(/\n/, $_, 6), split /\0/, $output;
     }
 
-    method _parse_diff_tree ($diff) {
-        my @keys = qw(modesrc modedst sha1src sha1dst status src dst);
-        my @ret;
-        while (@$diff and $diff->[0] =~ /^:\d+/) {
-            my $line = shift @$diff;
-            # see. man git-diff-tree for more info
-            # mode src, mode dst, sha1 src, sha1 dst, status, src[, dst]
-            my @vals = $line =~ /^:(\d+) (\d+) ($SHA1RE) ($SHA1RE) ([ACDMRTUX]\d*)\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+$/
-               if $line{sha1src};
-            @line{qw/status sim/} = $line{status} =~ /(R)(\d+)/
-                if $line{status} =~ /^R/;
-            push @ret, \%line;
-        }
-
-        return @ret;
-    }
-    method _parse_diff (@diff) {
-        my @ret;
-        for (@diff) {
-            # This regex is a little pathological.
-            if(m{^diff --git (a/(.*?)) (b/\2)}) {
-                push @ret, {
-                    head => $_,
-                    a    => $1,
-                    b    => $3,
-                    file => $2,
-                    diff => '',
-                };
-                next;
-            }
-
-            if(/^index (\w+)\.\.(\w+) (\d+)$/) {
-                @{$ret[-1]}{qw(index src dst mode)} = ($_, $1, $2, $3);
-                next
-            }
-
-            # XXX Somewhat hacky. Ahem.
-            $ret[@ret ? -1 : 0]{diff} .= "$_\n";
-        }
-
-        return @ret;
-    }
-
-    # 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 --
-    method _raw_diff (@args) {
-        return $self->run_cmd_list(
-            qw(diff-tree -r -M --no-commit-id --full-index),
-            @args
-        );
-    }
-
 =head1 SEE ALSO
 
 L<Gitalist::Git::Util> L<Gitalist::Git::Object>