Improved patchset display.

Big improvement: the ability to view a single patchset. Ultimate goal is
to get this display to look similar/identical to a commit message, so
that this page becomes the one-stop shop when it comes to a commit
(instead of linking to the various diff pages in commit e-mails for each
file, we instead link to a single patchset page for each commit). Input
and/or tweaking of the display of this page would be great.

Additionally, tweak error display on unavailable patchset, link to the
checkout page for file name (instead of patchset page), and add full
revision tooltip information to revision links.

diff --git a/chora/patchsets.php b/chora/patchsets.php
index 3dfea65..c454ebf 100644 (file)
--- a/chora/patchsets.php
+++ b/chora/patchsets.php
@@ -24,6 +24,9 @@ if (!$GLOBALS['VC']->hasFeature('patchsets')) {
 $ps_opts = array();
 if ($ps_id = Horde_Util::getFormData('ps')) {
     $ps_opts['range'] = array($ps_id);
+    $title = sprintf(_("Patchset (%s) for %s"), $ps_id, $where);
+} else {
+    $title = sprintf(_("Patchsets for %s"), $where);
 }
 
 try {
@@ -33,7 +36,10 @@ try {
     Chora::fatal($e);
 }
 
-$title = sprintf(_("Patchsets for %s"), $where);
+if (empty($patchsets)) {
+    Chora::fatal(_("Patchset Not Found"), '400 Bad Request');
+}
+
 $extraLink = Chora::getFileViews($where, 'patchsets');
 
 Horde::addScriptFile('prototype.js', 'horde', true);
@@ -51,35 +57,36 @@ require CHORA_TEMPLATES . '/menu.inc';
 require CHORA_TEMPLATES . '/headerbar.inc';
 require CHORA_TEMPLATES . '/patchsets/header.inc';
 
+if (!$ps_id) {
+    require CHORA_TEMPLATES . '/patchsets/header_table.inc';
+}
+
+$diff_img = Horde::img('diff.png', _("Diff"));
+
 reset($patchsets);
 while (list($id, $patchset) = each($patchsets)) {
-    // The diff should be from the top of the source tree so as to
-    // get all files.
-    // @TODO: Fix this (support needs to be written in diff page)
-    // $patchset_link = Horde::link(Chora::url('diff', substr($where, 0, strpos($where, '/', 1)), array('r1' => $id - 1, 'r2' => $id, 't' => 'unified'))) . $VC->abbrev($id) . '</a>';
-    $patchset_link = htmlspecialchars($VC->abbrev($id));
+    $patchset_link = Horde::link(Chora::url('patchsets', $where, array('ps' => $id)), sprintf("Patchset for %s", $id)) . htmlspecialchars($VC->abbrev($id)) . '</a>';
 
     $files = $tags = array();
-    $diff_img = Horde::img('diff.png', _("Diff"));
 
     foreach ($patchset['members'] as $member) {
         $file = array();
 
-        $file['file'] = Horde::link(Chora::url('patchsets', $member['file'])) . htmlspecialchars($member['file']) . '</a>';
+        $file['file'] = Horde::link(Chora::url('co', $member['file'])) . htmlspecialchars($member['file']) . '</a>';
 
         if ($member['status'] == Horde_Vcs_Patchset::ADDED) {
             $file['from'] = '<ins>' . _("New File") . '</ins>';
             $file['diff'] = '';
         } else {
-            $file['from'] = Horde::link(Chora::url('co', $member['file'], array('r' => $member['from']))) . htmlspecialchars($VC->abbrev($member['from'])) . '</a>';
-            $file['diff'] = Horde::link(Chora::url('diff', $member['file'], array('r1' => $member['from'], 'r2' => $member['to']))) . ' ' . $diff_img . '</a>';
+            $file['from'] = Horde::link(Chora::url('co', $member['file'], array('r' => $member['from'])), $member['from']) . htmlspecialchars($VC->abbrev($member['from'])) . '</a>';
+            $file['diff'] = Horde::link(Chora::url('diff', $member['file'], array('r1' => $member['from'], 'r2' => $member['to'])), _("Diff")) . ' ' . $diff_img . '</a>';
         }
 
         if ($member['status'] == Horde_Vcs_Patchset::DELETED) {
             $file['to'] = '<del>' . _("Deleted") . '</del>';
             $file['diff'] = '';
         } else {
-            $file['to'] = Horde::link(Chora::url('co', $member['file'], array('r' => $member['to']))) . htmlspecialchars($VC->abbrev($member['to'])) . '</a>';
+            $file['to'] = Horde::link(Chora::url('co', $member['file'], array('r' => $member['to'])), $member['to']) . htmlspecialchars($VC->abbrev($member['to'])) . '</a>';
         }
 
         $files[] = $file;
@@ -98,8 +105,14 @@ while (list($id, $patchset) = each($patchsets)) {
         $tags = array_merge($tags, $patchset['tag']);
     }
 
-    require CHORA_TEMPLATES . '/patchsets/ps.inc';
+    if ($ps_id) {
+        require CHORA_TEMPLATES . '/patchsets/ps_single.inc';
+    } else {
+        require CHORA_TEMPLATES . '/patchsets/ps.inc';
+    }
 }
 
-require CHORA_TEMPLATES . '/patchsets/footer.inc';
+if (!$ps_id) {
+    require CHORA_TEMPLATES . '/patchsets/footer.inc';
+}
 require $registry->get('templates', 'horde') . '/common-footer.inc';

diff --git a/chora/templates/patchsets/header.inc b/chora/templates/patchsets/header.inc
index e9a8c78..b3065f3 100644 (file)
--- a/chora/templates/patchsets/header.inc
+++ b/chora/templates/patchsets/header.inc
@@ -10,15 +10,3 @@
   </td>
  </tr>
 </table>
-
-<table class="revlog striped sortable" id="patchsets" cellspacing="0">
-<thead>
- <tr class="item leftAlign">
-  <th><?php echo _("Patchset") ?></th>
-  <th class="sortup"><?php echo _("Date") ?></th>
-  <th><?php echo _("Author") ?></th>
-  <th class="nosort"><?php echo _("Files") ?></th>
-  <th class="nosort"><?php echo _("Log Message") ?></th>
- </tr>
-</thead>
-<tbody id="patchsets_body">

diff --git a/chora/templates/patchsets/header_table.inc b/chora/templates/patchsets/header_table.inc
new file mode 100644 (file)
index 0000000..fbba0e1
--- /dev/null
+++ b/chora/templates/patchsets/header_table.inc
@@ -0,0 +1,11 @@
+<table class="revlog striped sortable" id="patchsets" cellspacing="0">
+<thead>
+ <tr class="item leftAlign">
+  <th><?php echo _("Patchset") ?></th>
+  <th class="sortup"><?php echo _("Date") ?></th>
+  <th><?php echo _("Author") ?></th>
+  <th class="nosort"><?php echo _("Files") ?></th>
+  <th class="nosort"><?php echo _("Log Message") ?></th>
+ </tr>
+</thead>
+<tbody id="patchsets_body">

diff --git a/chora/templates/patchsets/ps_single.inc b/chora/templates/patchsets/ps_single.inc
new file mode 100644 (file)
index 0000000..c8eacbe
--- /dev/null
+++ b/chora/templates/patchsets/ps_single.inc
@@ -0,0 +1,30 @@
+<div class="singleps">
+ <span class="headerLabel"><?php echo _("Date") ?>:</span>
+ <span class="ago"><a title="<?php echo $readableDate ?>"><?php echo $commitDate ?></a></span>
+</div>
+
+<div class="singleps">
+ <span class="headerLabel"><?php echo _("Author") ?>:</span> <?php echo $author ?>
+</div>
+
+<?php if (!empty($tags)): ?>
+<div class="singleps">
+ <span class="headerLabel"><?php echo _("Tags") ?>:</span> <?php echo implode(', ', array_map('htmlspecialchars', $tags)) ?>
+</div>
+<?php endif; ?>
+
+<div class="singleps">
+ <span class="headerLabel"><?php echo _("Log") ?>:</span>
+</div>
+
+<div class="fixed singlepslog"><?php echo $logMessage ?></div>
+
+<div class="singleps">
+ <span class="headerLabel"><?php echo _("Files") ?>:</span>
+</div>
+
+<ul class="singlepsfiles">
+<?php foreach ($files as $file): ?>
+ <li><?php echo $file['file'] . ': ' . $file['from'] . ' -> ' . $file['to'] . ' ' . $file['diff'] ?></li>
+<?php endforeach; ?>
+</ul>

diff --git a/chora/themes/screen.css b/chora/themes/screen.css
index d4d0e51..b060a32 100644 (file)
--- a/chora/themes/screen.css
+++ b/chora/themes/screen.css
@@ -151,6 +151,25 @@ h3.revision_log, h3.checkout {
     width: 35%;
 }
 
+.singleps span.headerLabel {
+    font-size: 110%;
+    font-weight: bold;
+    padding-right: 5px;
+}
+.singleps span.ago a:hover {
+    text-decoration: none;
+}
+.singlepslog {
+    padding: 10px 0 20px 20px;
+}
+ul.singlepsfiles {
+    list-style: none;
+    padding: 10px 0 0 20px;
+}
+ul.singlepsfiles li {
+    padding-bottom: 4px;
+}
+
 /* Checkout. */
 div.checkout {
     padding: 3px;

diff --git a/framework/Vcs/lib/Horde/Vcs/Git.php b/framework/Vcs/lib/Horde/Vcs/Git.php
index 2299b61..d288f55 100644 (file)
--- a/framework/Vcs/lib/Horde/Vcs/Git.php
+++ b/framework/Vcs/lib/Horde/Vcs/Git.php
@@ -797,7 +797,11 @@ class Horde_Vcs_Patchset_Git extends Horde_Vcs_Patchset
         }
 
         reset($revs);
-        while(list($rev, $log) = each($revs)) {
+        while (list($rev, $log) = each($revs)) {
+            if (empty($log)) {
+                continue;
+            }
+
             $this->_patchsets[$rev] = array(
                 'date' => $log->queryDate(),
                 'author' => $log->queryAuthor(),