Merge lp:~pauljnixon/loggerhead/more_columns into lp:loggerhead

[Paul Nixon]

Added "committer" and "comment" columns to Files and Inventory views.

Before: http://i.imgur.com/hQtr7.jpg
After: http://i.imgur.com/HishN.jpg

[Steve Kowalik (stevenk)]

Broadly, this looks good, but I have a few concerns. Firstly, comments should be formatted with a space after the hash, and secondly must be complete sentences. You have added some unneeded whitespace on line 29-30 of the diff. And lastly, since this branch does make some UI changes, I'd like to see at least one screenshot of the before and after view.

review: Needs Fixing (code)

[Paul Nixon (pauljnixon)]

Before: http://i.imgur.com/hQtr7.jpg
After: http://i.imgur.com/HishN.jpg

[Paul Nixon (pauljnixon)]

Does this need anything else?

[Steve Kowalik (stevenk)]

90 + # determine the last time something in the folder was changed

You missed one. :-)

Thanks for addressing my comments and posting screenshots -- this looks excellent, and I look forward to it landing.

review: Approve (code)

[Paul Nixon (pauljnixon)]

Abstaining from a review of my own code, which is already approved anyway. I invited myself to review accidentally.

review: Abstain

[Colin Watson (cjwatson)]

Sorry this has taken so long to land; I'm going through some old loggerhead merge proposals now. I was about to merge this since it's approved, but I thought the change to the _approximatedate function seemed odd, and possibly unintentional. Was there a reason you removed support for dates in the future from this? I think they can occur if the committer's clock is wrong, and in any case it shouldn't hurt to retain support for them.

review: Needs Information

[Paul Nixon (pauljnixon)]

Hello Colin:

Good point, I think I hadn't considered the possibility of incorrect
clocks. I'd like to keep the addition of months and years, though.

Thank you,
---Paul
Review: Needs Information

Sorry this has taken so long to land; I'm going through some old loggerhead
merge proposals now. I was about to merge this since it's approved, but I
thought the change to the _approximatedate function seemed odd, and
possibly unintentional. Was there a reason you removed support for dates
in the future from this? I think they can occur if the committer's clock
is wrong, and in any case it shouldn't hurt to retain support for them.
--
https://code.launchpad.net/~pauljnixon/loggerhead/more_columns/+merge/118188
You are the owner of lp:~pauljnixon/loggerhead/more_columns.

[Colin Watson (cjwatson)]

Sure, adding months and years makes sense in this context, I think. Can you push an adjustment, then, and I can get this merged for you?

[lp:~pauljnixon/loggerhead/more_columns]

479. By Paul Nixon on 2015-05-31

Re-added support for future dates in _approximatedate.

[Paul Nixon (pauljnixon)]

Hello Colin:

I've added support for future dates and pushed to
lp:~pauljnixon/loggerhead/more_columns . I'm sorry this took so long:
I no longer have access to the test environment that I originally
developed this on, so I had a bit of setup to do before I could start
working on this again.

Should I create a new merge request for the modifications, or resubmit
the old one somehow?

Thank you,
---Paul

On Tue, Mar 17, 2015 at 11:42 AM, Colin Watson <email address hidden> wrote:
> Sure, adding months and years makes sense in this context, I think. Can you push an adjustment, then, and I can get this merged for you?
> --
> https://code.launchpad.net/~pauljnixon/loggerhead/more_columns/+merge/118188
> You are the owner of lp:~pauljnixon/loggerhead/more_columns.

=== modified file 'loggerhead/apps/branch.py'
--- loggerhead/apps/branch.py	2012-02-08 01:50:02 +0000
+++ loggerhead/apps/branch.py	2015-05-31 16:20:47 +0000
@@ -93,6 +93,20 @@
         return History(
             self.branch, self.graph_cache,
             revinfo_disk_cache=revinfo_disk_cache, cache_key=self.friendly_name)
+    
+    # Before the addition of this method, clicking to sort by date from 
+    # within a branch caused a jump up to the top of that branch.
+    def sort_url(self, *args, **kw):
+        if isinstance(args[0], list):
+            args = args[0]
+        qs = []
+        for k, v in kw.iteritems():
+            if v is not None:
+                qs.append('%s=%s' % (k, urllib.quote(v)))
+        qs = '&'.join(qs)
+        path_info = self._path_info.strip('/').split('?')[0]
+        path_info += '?' + qs
+        return self._url_base + '/' + path_info
 
     def url(self, *args, **kw):
         if isinstance(args[0], list):
@@ -155,6 +169,7 @@
         if self.branch.get_config().get_user_option('http_serve') == 'False':
             raise httpexceptions.HTTPNotFound()
         self._url_base = environ['SCRIPT_NAME']
+        self._path_info = environ['PATH_INFO']
         self._static_url_base = environ.get('loggerhead.static.url')
         if self._static_url_base is None:
             self._static_url_base = self._url_base

=== modified file 'loggerhead/controllers/directory_ui.py'
--- loggerhead/controllers/directory_ui.py	2012-02-08 01:50:02 +0000
+++ loggerhead/controllers/directory_ui.py	2015-05-31 16:20:47 +0000
@@ -19,6 +19,7 @@
 import datetime
 import logging
 import stat
+import urllib
 
 from bzrlib import branch, errors
 
@@ -29,18 +30,17 @@
 class DirEntry(object):
 
     def __init__(self, dirname, parity, branch):
-        self.dirname = dirname
+        self.dirname = urllib.unquote(dirname)
         self.parity = parity
         self.branch = branch
         if branch is not None:
             # If a branch is empty, bzr raises an exception when trying this
             try:
-                self.last_change = datetime.datetime.utcfromtimestamp(
-                    branch.repository.get_revision(
-                        branch.last_revision()).timestamp)
+                self.last_revision = branch.repository.get_revision(branch.last_revision())
+                self.last_change_time = datetime.datetime.utcfromtimestamp(self.last_revision.timestamp)
             except:
-                self.last_change = None
-
+                self.last_revision = None
+                self.last_change_time = None
 
 class DirectoryUI(TemplatedBranchView):
     """

=== modified file 'loggerhead/controllers/inventory_ui.py'
--- loggerhead/controllers/inventory_ui.py	2013-03-27 05:29:14 +0000
+++ loggerhead/controllers/inventory_ui.py	2015-05-31 16:20:47 +0000
@@ -70,15 +70,27 @@
             change_dict[change.revid] = change
 
         for filename, entry in dir_ie.children.iteritems():
+            revid = entry.revision
+            file_timestamp = change_dict[revid].timestamp
             pathname = filename
-            if entry.kind == 'directory':
-                pathname += '/'
+            contents_changed_rev = None
             if path == '':
                 absolutepath = pathname
             else:
                 absolutepath = path + '/' + pathname
-            revid = entry.revision
-
+            if entry.kind == 'directory':
+                # determine the last time something in the folder was changed
+                pathname += '/'
+                sub_dir_id = inv.path2id(absolutepath)
+                sub_dir_ie = inv[sub_dir_id]
+                sub_revid_set = set()
+                for sub_filename, sub_entry in sub_dir_ie.children.iteritems():
+                    sub_revid_set.add(sub_entry.revision)
+                for change in self._history.get_changes(list(sub_revid_set)):
+                    change_dict[change.revid] = change
+                    if(change.timestamp > file_timestamp):
+                        revid = change.revid
+                        file_timestamp = change.timestamp
             # TODO: For the JSON rendering, this inlines the "change" aka
             # revision information attached to each file. Consider either
             # pulling this out as a separate changes dict, or possibly just
@@ -88,7 +100,7 @@
                 filename=filename, executable=entry.executable,
                 kind=entry.kind, absolutepath=absolutepath,
                 file_id=entry.file_id, size=entry.text_size, revid=revid,
-                change=change_dict[revid])
+                change=change_dict[revid], contents_changed=contents_changed_rev)
             file_list.append(file)
 
         if sort_type == 'filename':
@@ -96,10 +108,11 @@
         elif sort_type == 'size':
             file_list.sort(key=lambda x: x.size)
         elif sort_type == 'date':
-            file_list.sort(key=lambda x: x.change.date)
-
-        # Always sort directories first.
-        file_list.sort(key=lambda x: x.kind != 'directory')
+            file_list.sort(key=lambda x: x.change.date, reverse=True)
+        
+        if sort_type != 'date':
+        # Don't always sort directories first.
+            file_list.sort(key=lambda x: x.kind != 'directory')
 
         return file_list
 

=== modified file 'loggerhead/history.py'
--- loggerhead/history.py	2012-02-08 01:50:02 +0000
+++ loggerhead/history.py	2015-05-31 16:20:47 +0000
@@ -722,6 +722,7 @@
             'revid': revision.revision_id,
             'date': datetime.datetime.fromtimestamp(revision.timestamp),
             'utc_date': datetime.datetime.utcfromtimestamp(revision.timestamp),
+            'timestamp': revision.timestamp,
             'committer': revision.committer,
             'authors': revision.get_apparent_authors(),
             'branch_nick': revision.properties.get('branch-nick', None),

=== modified file 'loggerhead/static/css/global.css'
--- loggerhead/static/css/global.css	2015-03-17 11:38:32 +0000
+++ loggerhead/static/css/global.css	2015-05-31 16:20:47 +0000
@@ -230,11 +230,11 @@
     }
 th.datecell,
 td.date {
-    width: 180px;
+    width: 100px;
     }
 th.timedate,
 td.timedate2 {
-    width: 80px;
+    width: 60px;
     }
 th.downloadcell,
 td.downr {

=== modified file 'loggerhead/static/css/view.css'
--- loggerhead/static/css/view.css	2012-03-19 04:37:07 +0000
+++ loggerhead/static/css/view.css	2015-05-31 16:20:47 +0000
@@ -5,8 +5,7 @@
     }
 .viewRev {
     border-right: none;
-    white-space: nowrap;
-    width: 1px;
+    width: 20%;
     padding: .3em .6em;
     }
 .viewComm,

=== modified file 'loggerhead/templates/directory.pt'
--- loggerhead/templates/directory.pt	2011-02-17 19:51:25 +0000
+++ loggerhead/templates/directory.pt	2015-05-31 16:20:47 +0000
@@ -17,16 +17,16 @@
       <div >
         <table id="logentries">
           <tr class="logheader">
-            <th class="summarycell" colspan="2">Filename</th>
-            <th class="datecell">Latest Rev</th>
+            <th class="datecell">Filename</th>
+            <th class="timedate">Latest Rev</th>
             <th class="datecell">Last Changed</th>
+			<th class="datecell">Committer</th>
+            <th class="summarycell">Comment</th>
           </tr>
 
           <tr class="blueRow0" tal:condition="python:name != '/'">
-            <td class="icocell">
+            <td class="summcell" colspan="5">
                 <a href="../"><img tal:attributes="src python:static_url('/static/images/ico_folder_up.gif')" /></a>
-            </td>
-            <td class="summcell" colspan="3">
                 <a href="../">..</a>
             </td>
           </tr>
@@ -34,32 +34,31 @@
 
             <tal:branch-row tal:condition="dir/branch">
               <tr tal:attributes="class string:blueRow${dir/parity}">
-                <td class="icocell">
+                <td class="date">
                     <a tal:attributes="href string:${dir/dirname}/files">
                         <img tal:attributes="src python:static_url('/static/images/ico_branch.gif')" alt="Branch" />
                     </a>
-                </td>
-                <td class="autcell">
                   <a tal:attributes="href string:${dir/dirname}/files" tal:content="dir/dirname" /></td>
                 <td class="date">
                   <a tal:attributes="href string:${dir/dirname}/revision/${dir/branch/revno};
                                      title string:Show revision ${dir/branch/revno}"
                      tal:content="dir/branch/revno"></a>
                 </td>
-                <td class="date" tal:content="python:util.date_time(dir.last_change)"></td>
+				<td class="date" tal:content="python:util._approximatedate(dir.last_change_time)"></td> 
+				<td class="date" tal:content="python:util.hide_email(dir.last_revision.committer) if dir.last_revision is not None else 'Nobody'"></td>
+                <td class="autcell" tal:content="python:dir.last_revision.message[:50] if dir.last_revision is not None else ''"></td>
               </tr>
+			  
             </tal:branch-row>
             <tal:non-branch-row tal:condition="not:dir/branch">
               <tr tal:attributes="class string:blueRow${dir/parity}">
-                <td class="icocell">
+                <td class="date">
                     <a tal:attributes="href string:${dir/dirname}/">
                         <img tal:attributes="src python:static_url('/static/images/ico_folder.gif')" alt="Folder" />
                     </a>
-                </td>
-                <td class="autcell">
                   <a tal:attributes="href string:${dir/dirname}/" tal:content="dir/dirname" /></td>
                 <td class="date"></td>
-                <td class="date" tal:content="dir/last_change"></td>
+                <td class="date" colspan="4" tal:content="dir/last_change"></td>
               </tr>
             </tal:non-branch-row>
           </tal:block>

=== modified file 'loggerhead/templates/inventory.pt'
--- loggerhead/templates/inventory.pt	2012-02-02 07:42:24 +0000
+++ loggerhead/templates/inventory.pt	2015-05-31 16:20:47 +0000
@@ -30,7 +30,7 @@
           <tal:no-link condition="not: branch/branch_link">
             <span metal:use-macro="breadcrumbs/directory"></span>
           </tal:no-link>
-          <span>: <span metal:use-macro="breadcrumbs/branch" /> (revision <tal:revno content="change/revno"></tal:revno>)</span>
+          <span>: <span metal:use-macro="breadcrumbs/branch" /> (Revision <tal:revno content="change/revno"></tal:revno>)</span>
         </div>
       </tal:block>
 
@@ -54,10 +54,12 @@
 
           <table id="logentries">
             <tr class="logheader">
-              <th class="summarycell"><a tal:attributes="href python:url(['/files', revid], sort='filename')">Filename</a></th>
-              <th class="datecell">Latest Rev</th>
-              <th class="datecell"><a tal:attributes="href python:url(['/files', revid], sort='date')">Last Changed</a></th>
-              <th class="timedate"><a tal:attributes="href python:url(['/files', revid], sort='size')">Size</a></th>
+              <th class="datecell"><a tal:attributes="href python:branch.sort_url(['/files', revid], sort='filename')">Filename</a></th>
+              <th class="timedate">Latest Rev</th>
+              <th class="datecell"><a tal:attributes="href python:branch.sort_url(['/files', revid], sort='date')">Last Changed</a></th>
+              <th class="datecell">Committer</th>
+              <th class="summarycell">Comment</th>
+              <th class="timedate"><a tal:attributes="href python:branch.sort_url(['/files', revid], sort='size')">Size</a></th>
               <th class="expandcell"></th>
               <th class="expandcell"></th>
             </tr>
@@ -68,11 +70,15 @@
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_folder_up.gif')" />..</a>
                 </td>
             </tr>
+          <tr class="blueRow0" tal:condition="python:updir is None and name != '/'">
+            <td class="summcell" colspan="6">
+                <a tal:attributes="href python:'/'.join(branch.friendly_name.split('/')[:-1])"><img tal:attributes="src python:static_url('/static/images/ico_folder_up.gif')" />..</a></td>
+          </tr>
 
             <!-- Show this if it's a directory -->
             <tal:block repeat="file filelist">
             <tr tal:attributes="class string:blueRow${repeat/file/even}" tal:condition="python:file.kind=='directory'">
-                <td class="autcell"><a tal:attributes="href python:url(['/files', revno_url, file.absolutepath])">
+                <td class="date"><a tal:attributes="href python:url(['/files', revno_url, file.absolutepath])">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_folder.gif');
                                              title string:Go to ${file/filename}" />
                     </a>
@@ -84,8 +90,10 @@
                                                      title string:Show revision ${file/change/revno}"
                                      tal:content="file/change/revno"></a>
                  </td>
-                 <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
-                 <td class="timedate2"></td>
+                 <td class="date" tal:content="python:util._approximatedate(file.change.utc_date)"></td> 
+                 <td class="date" tal:content="python:util.hide_email(file.change.committer)"></td>
+                 <td class="autcell" tal:content="python:file.change.comment[:50]"></td>
+                 <td class="timedate2"></td><!-- not showing sizes of folders -->
                  <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
                                                      title string:Show revision ${file/change/revno}">
                                       <img tal:attributes="src python:branch.static_url('/static/images/ico_planilla.gif')" alt="Diff" />
@@ -96,7 +104,7 @@
 
             <!-- Show this if it's a symlink -->
             <tr tal:attributes="class string:blueRow${repeat/file/even}" tal:condition="python:file.kind=='symlink'">
-                <td class="autcell"><a tal:attributes="href python:url(['/view', change.revno, file.absolutepath])">
+                <td class="date"><a tal:attributes="href python:url(['/view', change.revno, file.absolutepath])">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_file_flecha.gif')" alt="Symlink" />
                     </a>
 
@@ -107,7 +115,9 @@
                                                      title string:Show revision ${file/change/revno}"
                                      tal:content="file/change/revno"></a>
                  </td>
-                 <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
+                 <td class="date" tal:content="python:util._approximatedate(file.change.utc_date)"></td> 
+                 <td class="date" tal:content="python:util.hide_email(file.change.committer)"></td>
+                 <td class="autcell" tal:content="python:file.change.comment[:50]"></td>
                  <td class="timedate2">.</td>
                  <td class="expcell"><a tal:attributes="href python:url(['/changes'], start_revid=start_revid, filter_file_id=file.file_id);
                                                         title string:Show revision ${file/change/revno}">
@@ -120,7 +130,7 @@
 
             <!-- Show this if it's a regular file -->
             <tr tal:attributes="class string:blueRow${repeat/file/even}" tal:condition="python:file.kind=='file'">
-                <td class="autcell"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath])">
+                <td class="date"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath])">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_file.gif');
                                              title string:View ${file/filename}"
                              tal:condition="python:file.executable is False" />
@@ -136,7 +146,10 @@
                                                   title string:Show revision ${file/change/revno}"
                                   tal:content="file/change/revno"></a>
               </td>
-              <td class="date" tal:content="python:util.date_time(file.change.utc_date)"></td>
+                 <td class="date" tal:content="python:util._approximatedate(file.change.utc_date)"></td> 
+                 <td class="date" tal:content="python:util.hide_email(file.change.committer)"></td>
+                 <td class="autcell" tal:content="python:file.change.comment[:50]"></td>
+
               <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>
               <td class="expcell"><a tal:attributes="href python:url(['/view', revno_url, file.absolutepath]);
                                                      title string:View ${file/filename}">

=== modified file 'loggerhead/util.py'
--- loggerhead/util.py	2012-04-28 00:57:06 +0000
+++ loggerhead/util.py	2015-05-31 16:20:47 +0000
@@ -73,12 +73,13 @@
 
 
 def _approximatedate(date):
-    delta = datetime.datetime.now() - date
-    if abs(delta) > datetime.timedelta(1, 0, 0):
-        # far in the past or future, display the date
-        return date_day(date)
+    if date is None:
+        return 'Never'
+    delta = datetime.datetime.utcnow() - date
     future = delta < datetime.timedelta(0, 0, 0)
     delta = abs(delta)
+    years = delta.days / 365
+    months = delta.days / 30 # This is approximate.
     days = delta.days
     hours = delta.seconds / 3600
     minutes = (delta.seconds - (3600*hours)) / 60
@@ -86,7 +87,13 @@
     result = ''
     if future:
         result += 'in '
-    if days != 0:
+    if years != 0:
+        amount = years
+        unit = 'year'
+    elif months != 0:
+        amount = months
+        unit = 'month'
+    elif days != 0:
         amount = days
         unit = 'day'
     elif hours != 0:
@@ -103,7 +110,7 @@
     result += '%s %s' % (amount, unit)
     if not future:
         result += ' ago'
-        return result
+    return result
 
 
 def _wrap_with_date_time_title(date, formatted_date):