Fix bug where diff didn't distinguish between sha1s and paths.
Dan Brook [Sun, 28 Mar 2010 20:23:49 +0000 (21:23 +0100)]
* The current approach to /diff + args is rather fragile, better suggestions welcome.
* Also fixed a link and made a few template tweaks.
* Added a redirect for /repo/tree to /repo/HEAD/tree.

lib/Gitalist/Controller/Repository.pm
lib/Gitalist/URIStructure/Ref.pm
root/_diff_tree.tt2
root/_tree.tt2 [deleted file]
root/inc/chroma_hash.tt2
root/ref/tree.tt2

index 410f0a2..0aba16c 100644 (file)
@@ -39,21 +39,16 @@ sub search : Chained('find') Args(0) {
   );
 }
 
-=head2 reflog
+=head2 tree
 
-Expose the local reflog. This may go away.
+Provide a simple redirect to C</ref/tree>.
 
 =cut
 
-sub reflog : Chained('find') Args(0) {
-  my ( $self, $c ) = @_;
-  my @log = $c->stash->{Repository}->reflog(
-      '--since=yesterday'
-  );
-
-  $c->stash(
-      log    => \@log,
-  );
+sub tree : Chained('find') Args(0) {
+    my($self, $c) = @_;
+    $c->res->redirect($c->uri_for_action('/ref/tree', [$c->stash->{Repository}->name, 'HEAD']));
+    $c->res->status(301);
 }
 
 =head2 atom
index 60fc6df..c1cab5c 100644 (file)
@@ -3,6 +3,8 @@ use MooseX::MethodAttributes::Role;
 use Moose::Autobox;
 use namespace::autoclean;
 
+use Gitalist::Git::Types qw/SHA1/;
+
 requires 'base';
 
 with qw/
@@ -24,22 +26,29 @@ sub find : Chained('base') PathPart('') CaptureArgs(1) {
 
 sub diff : Chained('find') CaptureArgs(0) {}
 
-sub diff_fancy : Chained('diff') PathPart('') Args() {
-    my($self, $c, $comparison, @rest) = @_;
+sub _set_diff_args {
+    my($self, $c, @rest) = @_;
+
     # FIXME - This ain't pretty
-    $c->stash(parent   => $comparison)
-      if $comparison;
-    $c->stash(filename => $rest[0])
+    $c->stash(parent   => shift @rest)
+       if @rest == 2
+       # Check that the single arg is unlikely to be a path.
+       or @rest && to_SHA1($rest[0]) && $c->stash->{Repository}->get_object_or_head($rest[0]);
+    $c->stash(filename => $rest[-1])
       if @rest;
 }
 
+sub diff_fancy : Chained('diff') PathPart('') Args() {
+    my($self, $c, @rest) = @_;
+
+    $self->_set_diff_args($c, @rest);
+ }
+
 sub diff_plain : Chained('diff') PathPart('plain') Args() {
     my($self, $c, $comparison, @rest) = @_;
-    # FIXME - This ain't pretty
-    $c->stash(parent   => $comparison)
-      if $comparison;
-    $c->stash(filename => $rest[0])
-      if @rest;
+
+    $self->_set_diff_args($c, @rest);
+
     $c->stash(no_wrapper => 1);
     $c->response->content_type('text/plain; charset=utf-8');
 }
index 2fc0ce3..7126a75 100755 (executable)
@@ -1,4 +1,4 @@
-<h2>[% diff_tree.size %] file[% "s" IF diff_tree.size > 1 %] in this commit <span>([%- Commit.sha1 || HEAD -%])</span></h2>
+<h2>[% diff_tree.size %] file[% "s" IF diff_tree.size > 1 %] in this diff <span>([%- Commit.sha1 || HEAD -%])</span></h2>
 
 <table class='diff-tree listing'>
  <thead>
@@ -12,7 +12,7 @@
   [% FOREACH line IN diff_tree -%]
   <tr>
    <td class='file-name'>
-    [% line.file %]
+    <a href="#diff[% loop.count %]">[% line.file %]</a>
    </td>
    <td class='status'>
     [%
@@ -27,8 +27,8 @@
     %]
    </td>
    <td class='action-list'>
-     [% IF !line.is_new %]<a href="[% c.uri_for_action("/ref/diff_fancy", [Repository.name, Commit.sha1], line.file.to_path) %]#diff[% loop.count %]" title="Difference" class="button diff">diff</a>[% END %]
-     [% IF !line.is_new %]<a href="[% c.uri_for("/ref/shortlog", [Repository.name, Commit.sha1], line.file.to_path) %]" title="History (Short Log)" class="button history">history</a>[% END %]
+     [% IF !line.is_new %]<a href="[% c.uri_for_action("/ref/diff_fancy", [Repository.name, Commit.sha1], line.file.to_path) %]" title="Difference" class="button diff">diff</a>[% END %]
+     [% IF !line.is_new %]<a href="[% c.uri_for_action("/ref/history", [Repository.name, Commit.sha1], line.file.to_path) %]" title="History (Short Log)" class="button history">history</a>[% END %]
        <a href="[% c.uri_for_action("/ref/blob", [Repository.name, Commit.sha1], line.file.to_path) %]" title="Blob" class="button blob">blob</a>
    </td>
   </tr>
diff --git a/root/_tree.tt2 b/root/_tree.tt2
deleted file mode 100644 (file)
index 6a96cda..0000000
+++ /dev/null
@@ -1,39 +0,0 @@
-<table class='tree listing'>
- <thead>
-  <tr>
-   <th>mode</th>
-   <th>file</th>
-   <th>actions</th>
-  </tr>
- </thead>
- <tfoot>
-  <tr>
-   <td>mode</td>
-   <td>file</td>
-   <td>actions</td>
-  </tr>
- </tfoot>
-
- <tbody>
-  [% FOREACH item IN tree_list %]
-  <tr>
-   <td class='file-mode'>[% item.modestr %]</td>
-   [% theact = item.type == 'tree' ? 'tree' : 'blob' -%]
-   [% fullpath = path ? path _ '/' _ item.file : item.file %]
-   <td class='file-name'>
-    <a href="[% c.uri_for(theact, {h=item.sha1, hb=commit.sha1, f=fullpath}) %]">[% item.file %]</a>
-   </td>
-   <td class='action-list'>
-     <a href="[% c.uri_for(theact, {h=item.sha1, hb=commit.sha1, f=fullpath}) %]">[% theact %]</a>
-         [% IF item.type == 'blob' %]
-         <a href="[% c.uri_for('blame', {h=commit.sha1, hb=commit.sha1, f=fullpath}) %]">blame</a>
-         [% END %]
-     <a href="[% c.uri_for('history', {h=item.sha1, hb=commit.sha1, f=fullpath}) %]">history</a>
-     [% IF item.type == 'blob' %]
-     <a href="[% c.uri_for('raw', {hb=commit.sha1, f=fullpath}) %]">raw</a>
-     [% END %]
-   </td>
-  </tr>
-  [% END %]
- </tbody>
-</table>
index d201d96..b07a1ea 100755 (executable)
@@ -7,7 +7,7 @@
     END;
 -%]
 
-<div class='button sha1_holder[% "_invert" IF loop.count % 2 %]' style="background-color:#[% sha1part %]"></div>
+<div class='button sha1_holder[% "_invert" IF loop.count && loop.count % 2 %]' style="background-color:#[% sha1part %]"></div>
 [% IF !hide_sha1_output %]
 <div class="sha1_label">[% sha1part %]</div>
 [% END %]
index 33ed034..b477e0c 100755 (executable)
@@ -1,6 +1,3 @@
-
-
-
   [%
     IF path;
       INCLUDE 'nav/path.tt2' filename = path;
@@ -9,6 +6,3 @@
     subinclude('/fragment/ref/tree', c.req.captures, c.req.args.to_path);
   %]
 
-   <div class='commit-message'>
-  [% short_cmt(commit.comment) | html %] ...
-  </div>