loggerhead

Merge lp:~beuno/loggerhead/abstract_paths into lp:loggerhead

abstract_paths
Merge into trunk-rich

Proposed by Martin Albisetti on 2008-10-25

Status:

Merged

Merged at revision:

233

Proposed branch:

lp:~beuno/loggerhead/abstract_paths

Merge into:

lp:loggerhead

To merge this branch:

bzr merge lp:~beuno/loggerhead/abstract_paths

Related bugs:

Bug #128926: code browser is not very linkable	Undecided	Fix Released
Bug #260363: unable to generate a link showing a file changes based on its path	Medium	Fix Released
Bug #269365: no way to link to 'HEAD of PATH'	Medium	Fix Released

Link a bug report

Reviewer	Date Requested	Status
Paul Hummer (community)	2008-10-25	Needs Fixing on 2008-10-25
Loggerhead Team	2008-10-25	Pending
Review via email: mp+1414@code.launchpad.net

Revision history for this message

Martin Albisetti (beuno) wrote on 2008-10-25:

The branch refactors a few cruft, and implements paths instead of file_ids to navigate through Loggerhead

Revision history for this message

Paul Hummer (rockstar) wrote on 2008-10-25:

Download full text (18.9 KiB)

+ revid = None
+ if h is not None:
+ if len(args) > 0:
+ revid = h.fix_revid(args[0])
+ else:
+ revid = h.last_revid
+
+ path = None
+ if len(args) > 1:
+ path = '/'.join(args[1:])
+
         vals = {
             'static_url': self._branch.static_url,
             'branch': self._branch,
@@ -81,7 +92,7 @@ class TemplatedBranchView(object):
         }
         vals.update(templatefunctions)
         headers = {}
- vals.update(self.get_values(h, args, kw, headers))
+ vals.update(self.get_values(h, revid, path, kwargs, headers))

         self.log.info('Getting information for %s: %r secs' % (
             self.__class__.__name__, time.time() - z,))
@@ -93,6 +104,7 @@ class TemplatedBranchView(object):
         w = BufferingWriter(writer, 8192)
         template.expand_into(w, **vals)
         w.flush()
- self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % (
- self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 100.0*w.bytes_saved/w.bytes))
+ self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' %
+ (self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved,
+ 100.0*w.bytes_saved/w.bytes))
         return []

template_path = 'loggerhead.templates.annotate'

- def get_values(self, h, args, kw, headers):
- if len(args) > 0:
- revid = h.fix_revid(args[0])
- else:
- revid = h.last_revid
-
- path = None
- if len(args) > 1:
- path = '/'.join(args[1:])
- if not path.startswith('/'):
- path = '/' + path
+ def get_values(self, h, revid, path, kwargs, headers):

- file_id = kw.get('file_id', None)
+ revid = h.fix_revid(revid)
+ file_id = kwargs.get('file_id', None)
if (file_id is None) and (path is None):
raise HTTPBadRequest('No file_id or filename provided to annotate')

=== modified file 'loggerhead/controllers/atom_ui.py'
--- loggerhead/controllers/atom_ui.py 2008-07-22 00:40:03 +0000
+++ loggerhead/controllers/atom_ui.py 2008-10-25 18:13:27 +0000
@@ -24,7 +24,...

=== modified file 'loggerhead/controllers/__init__.py'
--- loggerhead/controllers/__init__.py	2008-09-30 02:04:14 +0000
+++ loggerhead/controllers/__init__.py	2008-10-25 18:13:27 +0000
@@ -63,8 +63,8 @@ class TemplatedBranchView(object):
     def __call__(self, environ, start_response):
         z = time.time()
         h = self._history
-        kw = dict(parse_querystring(environ))
-        util.set_context(kw)
+        kwargs = dict(parse_querystring(environ))
+        util.set_context(kwargs)
         args = []
         while 1:
             arg = path_info_pop(environ)
@@ -72,6 +72,17 @@ class TemplatedBranchView(object):
                 break
             args.append(arg)
 
+        revid = None
+        if h is not None:
+            if len(args) > 0:
+                revid = h.fix_revid(args[0])
+            else:
+                revid = h.last_revid
+
+        path = None
+        if len(args) > 1:
+            path = '/'.join(args[1:])
+
         vals = {
             'static_url': self._branch.static_url,
             'branch': self._branch,
@@ -81,7 +92,7 @@ class TemplatedBranchView(object):
         }
         vals.update(templatefunctions)
         headers = {}
-        vals.update(self.get_values(h, args, kw, headers))
+        vals.update(self.get_values(h, revid, path, kwargs, headers))
 
         self.log.info('Getting information for %s: %r secs' % (
             self.__class__.__name__, time.time() - z,))
@@ -93,6 +104,7 @@ class TemplatedBranchView(object):
         w = BufferingWriter(writer, 8192)
         template.expand_into(w, **vals)
         w.flush()
-        self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % (
-            self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 100.0*w.bytes_saved/w.bytes))
+        self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % 
+            (self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 
+                100.0*w.bytes_saved/w.bytes))
         return []

=== modified file 'loggerhead/controllers/annotate_ui.py'
--- loggerhead/controllers/annotate_ui.py	2008-09-09 22:42:27 +0000
+++ loggerhead/controllers/annotate_ui.py	2008-10-25 18:13:27 +0000
@@ -36,19 +36,10 @@ class AnnotateUI (TemplatedBranchView):
 
     template_path = 'loggerhead.templates.annotate'
 
-    def get_values(self, h, args, kw, headers):
-        if len(args) > 0:
-            revid = h.fix_revid(args[0])
-        else:
-            revid = h.last_revid
-
-        path = None
-        if len(args) > 1:
-            path = '/'.join(args[1:])
-            if not path.startswith('/'):
-                path = '/' + path
+    def get_values(self, h, revid, path, kwargs, headers):
 
-        file_id = kw.get('file_id', None)
+        revid = h.fix_revid(revid)
+        file_id = kwargs.get('file_id', None)
         if (file_id is None) and (path is None):
             raise HTTPBadRequest('No file_id or filename provided to annotate')

=== modified file 'loggerhead/controllers/atom_ui.py'
--- loggerhead/controllers/atom_ui.py	2008-07-22 00:40:03 +0000
+++ loggerhead/controllers/atom_ui.py	2008-10-25 18:13:27 +0000
@@ -24,7 +24,7 @@ class AtomUI (TemplatedBranchView):
 
     template_path = 'loggerhead.templates.atom'
 
-    def get_values(self, h, args, kw, headers):
+    def get_values(self, h, revid, path, kwargs, headers):
         pagesize = int(20)#self._branch.config.get('pagesize', '20'))
 
         revid_list = h.get_file_view(h.last_revid, None)

=== modified file 'loggerhead/controllers/changelog_ui.py'
--- loggerhead/controllers/changelog_ui.py	2008-08-16 21:53:52 +0000
+++ loggerhead/controllers/changelog_ui.py	2008-10-25 18:13:27 +0000
@@ -27,23 +27,22 @@ class ChangeLogUI(TemplatedBranchView):
 
     template_path = 'loggerhead.templates.changelog'
 
-    def get_values(self, h, args, kw, headers):
-        if args:
-            revid = h.fix_revid(args[0])
-        else:
-            revid = None
-
-        filter_file_id = kw.get('filter_file_id', None)
-        query = kw.get('q', None)
-        start_revid = h.fix_revid(kw.get('start_revid', None))
+    def get_values(self, h, revid, path, kwargs, headers):
+
+        filter_file_id = kwargs.get('filter_file_id', None)
+        query = kwargs.get('q', None)
+        start_revid = h.fix_revid(kwargs.get('start_revid', None))
         orig_start_revid = start_revid
         pagesize = 20#int(config.get('pagesize', '20'))
         search_failed = False
 
+        if filter_file_id is None and path is not None:
+            filter_file_id = h.get_file_id(revid, path)
+
         try:
             revid, start_revid, revid_list = h.get_view(
                 revid, start_revid, filter_file_id, query)
-            util.set_context(kw)
+            util.set_context(kwargs)
 
             if (query is not None) and (len(revid_list) == 0):
                 search_failed = True
@@ -74,18 +73,6 @@ class ChangeLogUI(TemplatedBranchView):
         # add parent & merge-point branch-nick info, in case it's useful
         h.get_branch_nicks(changes)
 
-        # does every change on this page have the same committer?  if so,
-        # tell the template to show committer info in the "details block"
-        # instead of on each line.
-        all_same_author = True
-
-        if changes:
-            author = changes[0].author
-            for e in changes[1:]:
-                if e.author != author:
-                    all_same_author = False
-                    break
-
         # Directory Breadcrumbs
         directory_breadcrumbs = (
             util.directory_breadcrumbs(
@@ -105,7 +92,6 @@ class ChangeLogUI(TemplatedBranchView):
             'viewing_from': (orig_start_revid is not None) and (orig_start_revid != h.last_revid),
             'query': query,
             'search_failed': search_failed,
-            'all_same_author': all_same_author,
             'url': self._branch.context_url,
             'directory_breadcrumbs': directory_breadcrumbs,
         }

=== modified file 'loggerhead/controllers/directory_ui.py'
--- loggerhead/controllers/directory_ui.py	2008-09-30 02:57:08 +0000
+++ loggerhead/controllers/directory_ui.py	2008-10-25 18:13:27 +0000
@@ -57,7 +57,7 @@ class DirectoryUI(TemplatedBranchView):
         self._static_url_base = static_url_base
         self.log = logging.getLogger('')
 
-    def get_values(self, h, args, kwargs, response):
+    def get_values(self, h, revid, path, kwargs, response):
         listing = [d for d in os.listdir(self._path)
                    if not d.startswith('.')
                    and os.path.isdir(os.path.join(self._path, d))]

=== modified file 'loggerhead/controllers/inventory_ui.py'
--- loggerhead/controllers/inventory_ui.py	2008-10-01 22:23:24 +0000
+++ loggerhead/controllers/inventory_ui.py	2008-10-25 18:13:27 +0000
@@ -1,4 +1,5 @@
 #
+# Copyright (C) 2008  Canonical Ltd.
 # Copyright (C) 2006  Robey Pointer <robey@lag.net>
 # Copyright (C) 2006  Goffredo Baroncelli <kreijack@inwind.it>
 #
@@ -19,6 +20,7 @@
 
 import logging
 import posixpath
+import urllib
 
 from paste.httpexceptions import HTTPServerError
 
@@ -31,9 +33,10 @@ from loggerhead.controllers import Templ
 log = logging.getLogger("loggerhead.controllers")
 
 def dirname(path):
-    while path.endswith('/'):
-        path = path[:-1]
-    path = posixpath.dirname(path)
+    if path is not None:
+        while path.endswith('/'):
+            path = path[:-1]
+        path = urllib.quote(posixpath.dirname(path))
     return path
 
 
@@ -41,14 +44,42 @@ class InventoryUI(TemplatedBranchView):
 
     template_path = 'loggerhead.templates.inventory'
 
-    def get_values(self, h, args, kw, headers):
-        if len(args) > 0:
-            revid = h.fix_revid(args[0])
-        else:
-            revid = h.last_revid
+    def get_values(self, h, revid, path, kwargs, headers):
+
+        try:
+            inv = h.get_inventory(revid)
+        except:
+            self.log.exception('Exception fetching changes')
+            raise HTTPServerError('Could not fetch changes')
+
+        file_id = kwargs.get('file_id', None)
+        start_revid = kwargs.get('start_revid', None)
+        sort_type = kwargs.get('sort', None)
 
         # no navbar for revisions
         navigation = util.Container()
+
+        change = h.get_changes([ revid ])[0]
+        # add parent & merge-point branch-nick info, in case it's useful
+        h.get_branch_nicks([ change ])
+
+        if path is not None:
+            if not path.startswith('/'):
+                path = '/' + path
+            file_id = h.get_file_id(revid, path)
+        else:
+            path = inv.id2path(file_id)
+        
+        if file_id is None:
+            file_id = inv.root.file_id
+
+
+        idpath = inv.get_idpath(file_id)
+        if len(idpath) > 1:
+            updir = dirname(path)[1:]
+        else:
+            updir = None
+
         # Directory Breadcrumbs
         directory_breadcrumbs = util.directory_breadcrumbs(
                 self._branch.friendly_name,
@@ -62,27 +93,10 @@ class InventoryUI(TemplatedBranchView):
                 self.log.exception('Exception fetching changes')
                 raise HTTPServerError('Could not fetch changes')
 
-            file_id = kw.get('file_id', inv.root.file_id)
-            start_revid = kw.get('start_revid', None)
-            sort_type = kw.get('sort', None)
-
             change = h.get_changes([ revid ])[0]
             # add parent & merge-point branch-nick info, in case it's useful
             h.get_branch_nicks([ change ])
 
-            path = inv.id2path(file_id)
-            if not path.startswith('/'):
-                path = '/' + path
-            idpath = inv.get_idpath(file_id)
-            if len(idpath) > 1:
-                updir = dirname(path)
-                updir_file_id = idpath[-2]
-            else:
-                updir = None
-                updir_file_id = None
-            if updir == '/':
-                updir_file_id = None
-
             # Create breadcrumb trail for the path within the branch
             branch_breadcrumbs = util.branch_breadcrumbs(path, inv, 'files')
             filelist = h.get_filelist(inv, file_id, sort_type)
@@ -95,7 +109,6 @@ class InventoryUI(TemplatedBranchView):
             path = "/"
             idpath = None
             updir = None
-            updir_file_id = None
             branch_breadcrumbs = []
             filelist = []
 
@@ -107,7 +120,6 @@ class InventoryUI(TemplatedBranchView):
             'file_id': file_id,
             'path': path,
             'updir': updir,
-            'updir_file_id': updir_file_id,
             'filelist': filelist,
             'history': h,
             'posixpath': posixpath,

=== modified file 'loggerhead/controllers/revision_ui.py'
--- loggerhead/controllers/revision_ui.py	2008-08-16 21:53:52 +0000
+++ loggerhead/controllers/revision_ui.py	2008-10-25 18:13:27 +0000
@@ -30,21 +30,17 @@ class RevisionUI(TemplatedBranchView):
 
     template_path = 'loggerhead.templates.revision'
 
-    def get_values(self, h, args, kw, headers):
+    def get_values(self, h, revid, path, kwargs, headers):
 
-        if len(args) > 0:
-            revid = h.fix_revid(args[0])
-        else:
-            revid = None
-
-        filter_file_id = kw.get('filter_file_id', None)
-        start_revid = h.fix_revid(kw.get('start_revid', None))
-        query = kw.get('q', None)
-        remember = h.fix_revid(kw.get('remember', None))
-        compare_revid = h.fix_revid(kw.get('compare_revid', None))
+        filter_file_id = kwargs.get('filter_file_id', None)
+        start_revid = h.fix_revid(kwargs.get('start_revid', None))
+        query = kwargs.get('q', None)
+        remember = h.fix_revid(kwargs.get('remember', None))
+        compare_revid = h.fix_revid(kwargs.get('compare_revid', None))
 
         try:
-            revid, start_revid, revid_list = h.get_view(revid, start_revid, filter_file_id, query)
+            revid, start_revid, revid_list = h.get_view(revid, start_revid, 
+                                                        filter_file_id, query)
         except:
             self.log.exception('Exception fetching changes')
             raise HTTPServerError('Could not fetch changes')
@@ -68,7 +64,8 @@ class RevisionUI(TemplatedBranchView):
                 line_count += len(chunk.diff)
 
         # let's make side-by-side diff be the default
-        side_by_side = not kw.get('unified', False)
+        # FIXME: not currently in use. Should be
+        side_by_side = not kwargs.get('unified', False)
         if side_by_side:
             h.add_side_by_side([ change ])

=== modified file 'loggerhead/controllers/search_ui.py'
--- loggerhead/controllers/search_ui.py	2008-09-09 22:42:27 +0000
+++ loggerhead/controllers/search_ui.py	2008-10-25 18:13:27 +0000
@@ -28,7 +28,7 @@ class SearchUI(TemplatedBranchView):
 
     template_path = 'loggerhead.templates.search'
 
-    def get_values(self, h, args, kwargs, response):
+    def get_values(self, h, revid, path, kwargs, response):
         """
         Default method called from the search box as /search URL

=== modified file 'loggerhead/history.py'
--- loggerhead/history.py	2008-09-27 05:13:24 +0000
+++ loggerhead/history.py	2008-10-25 18:13:27 +0000
@@ -35,6 +35,7 @@ import re
 import textwrap
 import threading
 import time
+import urllib
 from StringIO import StringIO
 
 from loggerhead import search
@@ -796,12 +797,15 @@ class History (object):
             pathname = filename
             if entry.kind == 'directory':
                 pathname += '/'
-
+            if path == '':
+                absolutepath = pathname
+            else:
+            	absolutepath = urllib.quote(path + '/' + pathname)
             revid = entry.revision
 
             file = util.Container(
                 filename=filename, executable=entry.executable, kind=entry.kind,
-                pathname=pathname, file_id=entry.file_id, size=entry.text_size,
+                pathname=pathname, absolutepath=absolutepath, file_id=entry.file_id, size=entry.text_size,
                 revid=revid, change=change_dict[revid])
             file_list.append(file)

=== modified file 'loggerhead/templates/inventory.pt'
--- loggerhead/templates/inventory.pt	2008-10-01 22:15:10 +0000
+++ loggerhead/templates/inventory.pt	2008-10-25 18:13:27 +0000
@@ -53,9 +53,9 @@
               <th class="expandcell"></th>
             </tr>
 
-            <tr class="blueRow0" tal:condition="python:updir">
+            <tr class="blueRow0" tal:condition="python:updir is not None">
                 <td class="summcell" colspan="6">
-                    <a tal:attributes="href python:url(['/files', revid], file_id=updir_file_id)" class="link">
+                    <a tal:attributes="href python:url(['/files', change.revno, updir])" class="link">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_folder_up.gif')" />..</a>
                 </td>
             </tr>
@@ -63,12 +63,12 @@
             
             <tal:block repeat="file filelist">
             <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='directory'">
-                <td class="autcell"><a tal:attributes="href python:url(['/files', change.revno], file_id=file.file_id)">
+                <td class="autcell"><a tal:attributes="href python:url(['/files', change.revno], file.absolutepath)">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_folder.gif');
                                              title string:Go to ${file/filename}" />
                     </a>
 
-                 <a tal:attributes="href python:url(['/files', change.revno], file_id=file.file_id)" 
+                 <a tal:attributes="href python:url(['/files', change.revno, file.absolutepath])" 
                                         tal:content="file/filename" class="link"></a>
                  </td>
                  <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
@@ -87,11 +87,11 @@
 
             
             <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='symlink'">
-                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)">
+                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_flecha.gif')" alt="Symlink" />
                     </a>
 
-                 <a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)" 
+                 <a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])" 
                                         tal:content="file/filename" class="link"></a>
                  </td>
                  <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
@@ -111,7 +111,7 @@
 
             
             <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='file'">
-                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)">
+                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])">
                         <img tal:attributes="src python:branch.static_url('/static/images/ico_file.gif');
                                              title string:Annotate ${file/filename}"
                              tal:condition="python:file.executable is False" />
@@ -121,7 +121,7 @@
                              tal:condition="python:file.executable is True" alt="File" />
                     </a>
 
-              <a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)"
+              <a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])"
                                      tal:content="file/filename" class="link"></a></td>
               <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
                                                   title string:Show revision ${file/change/revno}"
@@ -129,7 +129,7 @@
               </td>
               <td class="date" tal:content="python:util.date_time(file.change.date)"></td>
               <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>
-              <td class="expcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id);
+              <td class="expcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath]);
                                                      title string:Annotate ${file/filename}">
                                       <img tal:attributes="src python:branch.static_url('/static/images/ico_planilla.gif')" alt="Diff" />
                                      </a>

Revision history for this message

Paul Hummer (rockstar) wrote on 2008-10-25:

Download full text (20.3 KiB)

Branch is good. I'd vote tweak on a few small, stylistic changes: 80 line
character breaks, the var name h should be history, and a single tab.

> === modified file 'loggerhead/controllers/__init__.py'
> --- loggerhead/controllers/__init__.py 2008-09-30 02:04:14 +0000
> +++ loggerhead/controllers/__init__.py 2008-10-25 18:13:27 +0000
> @@ -63,8 +63,8 @@ class TemplatedBranchView(object):
> def __call__(self, environ, start_response):
> z = time.time()
> h = self._history
> - kw = dict(parse_querystring(environ))
> - util.set_context(kw)
> + kwargs = dict(parse_querystring(environ))
> + util.set_context(kwargs)
> args = []
> while 1:
> arg = path_info_pop(environ)
> @@ -72,6 +72,17 @@ class TemplatedBranchView(object):
> break
> args.append(arg)
>
> + revid = None
> + if h is not None:
> + if len(args) > 0:
> + revid = h.fix_revid(args[0])
> + else:
> + revid = h.last_revid
> +
> + path = None
> + if len(args) > 1:
> + path = '/'.join(args[1:])
> +
> vals = {
> 'static_url': self._branch.static_url,
> 'branch': self._branch,
> @@ -81,7 +92,7 @@ class TemplatedBranchView(object):
> }
> vals.update(templatefunctions)
> headers = {}
> - vals.update(self.get_values(h, args, kw, headers))
> + vals.update(self.get_values(h, revid, path, kwargs, headers))
>
> self.log.info('Getting information for %s: %r secs' % (
> self.__class__.__name__, time.time() - z,))
> @@ -93,6 +104,7 @@ class TemplatedBranchView(object):
> w = BufferingWriter(writer, 8192)
> template.expand_into(w, **vals)
> w.flush()
> - self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % (
> - self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 100.0*w.bytes_saved/w.bytes))
> + self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' %
> + (self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved,
> + 100.0*w.bytes_saved/w.bytes))
> return []
>

Break at 80 characters please.

> === modified file 'loggerhead/controllers/annotate_ui.py'
> --- loggerhead/controllers/annotate_ui.py 2008-09-09 22:42:27 +0000
> +++ loggerhead/controllers/annotate_ui.py 2008-10-25 18:13:27 +0000
> @@ -36,19 +36,10 @@ class AnnotateUI (TemplatedBranchView):
>
> template_path = 'loggerhead.templates.annotate'
>
> - def get_values(self, h, args, kw, headers):
> - if len(args) > 0:
> - revid = h.fix_revid(args[0])
> - else:
> - revid = h.last_revid
> -
> - path = None
> - if len(args) > 1:
> - path = '/'.join(args[1:])
> - if not path.startswith('/'):
> - path = '/' + path
> + def get_values(self, h, revid, path, kwargs, headers):
>
> - file_id = kw.get('file_id', None)
> + revid = h.fix_revid(revid)
> + file_id = kwargs.get('file_id', None)
...

Branch is good.  I'd vote tweak on a few small, stylistic changes: 80 line
character breaks, the var name h should be history, and a single tab.

> === modified file 'loggerhead/controllers/__init__.py'
> --- loggerhead/controllers/__init__.py	2008-09-30 02:04:14 +0000
> +++ loggerhead/controllers/__init__.py	2008-10-25 18:13:27 +0000
> @@ -63,8 +63,8 @@ class TemplatedBranchView(object):
>      def __call__(self, environ, start_response):
>          z = time.time()
>          h = self._history
> -        kw = dict(parse_querystring(environ))
> -        util.set_context(kw)
> +        kwargs = dict(parse_querystring(environ))
> +        util.set_context(kwargs)
>          args = []
>          while 1:
>              arg = path_info_pop(environ)
> @@ -72,6 +72,17 @@ class TemplatedBranchView(object):
>                  break
>              args.append(arg)
>  
> +        revid = None
> +        if h is not None:
> +            if len(args) > 0:
> +                revid = h.fix_revid(args[0])
> +            else:
> +                revid = h.last_revid
> +
> +        path = None
> +        if len(args) > 1:
> +            path = '/'.join(args[1:])
> +
>          vals = {
>              'static_url': self._branch.static_url,
>              'branch': self._branch,
> @@ -81,7 +92,7 @@ class TemplatedBranchView(object):
>          }
>          vals.update(templatefunctions)
>          headers = {}
> -        vals.update(self.get_values(h, args, kw, headers))
> +        vals.update(self.get_values(h, revid, path, kwargs, headers))
>  
>          self.log.info('Getting information for %s: %r secs' % (
>              self.__class__.__name__, time.time() - z,))
> @@ -93,6 +104,7 @@ class TemplatedBranchView(object):
>          w = BufferingWriter(writer, 8192)
>          template.expand_into(w, **vals)
>          w.flush()
> -        self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % (
> -            self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 100.0*w.bytes_saved/w.bytes))
> +        self.log.info('Rendering %s: %r secs, %s bytes, %s (%2.1f%%) bytes saved' % 
> +            (self.__class__.__name__, time.time() - z, w.bytes, w.bytes_saved, 
> +                100.0*w.bytes_saved/w.bytes))
>          return []
>

Break at 80 characters please.

> === modified file 'loggerhead/controllers/annotate_ui.py'
> --- loggerhead/controllers/annotate_ui.py	2008-09-09 22:42:27 +0000
> +++ loggerhead/controllers/annotate_ui.py	2008-10-25 18:13:27 +0000
> @@ -36,19 +36,10 @@ class AnnotateUI (TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.annotate'
>  
> -    def get_values(self, h, args, kw, headers):
> -        if len(args) > 0:
> -            revid = h.fix_revid(args[0])
> -        else:
> -            revid = h.last_revid
> -
> -        path = None
> -        if len(args) > 1:
> -            path = '/'.join(args[1:])
> -            if not path.startswith('/'):
> -                path = '/' + path
> +    def get_values(self, h, revid, path, kwargs, headers):
>  
> -        file_id = kw.get('file_id', None)
> +        revid = h.fix_revid(revid)
> +        file_id = kwargs.get('file_id', None)
>          if (file_id is None) and (path is None):
>              raise HTTPBadRequest('No file_id or filename provided to annotate')
>  
>

Also, 80 char limit here as well.

> === modified file 'loggerhead/controllers/atom_ui.py'
> --- loggerhead/controllers/atom_ui.py	2008-07-22 00:40:03 +0000
> +++ loggerhead/controllers/atom_ui.py	2008-10-25 18:13:27 +0000
> @@ -24,7 +24,7 @@ class AtomUI (TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.atom'
>  
> -    def get_values(self, h, args, kw, headers):
> +    def get_values(self, h, revid, path, kwargs, headers):
>          pagesize = int(20)#self._branch.config.get('pagesize', '20'))
>  
>          revid_list = h.get_file_view(h.last_revid, None)
>

I'm not comfortable with the h arg.  Could it be more detailed.  I realize that
it was there before, but if we're going to go ahead and change the method
signature, it might be a good idea to change that as well.

> === modified file 'loggerhead/controllers/changelog_ui.py'
> --- loggerhead/controllers/changelog_ui.py	2008-08-16 21:53:52 +0000
> +++ loggerhead/controllers/changelog_ui.py	2008-10-25 18:13:27 +0000
> @@ -27,23 +27,22 @@ class ChangeLogUI(TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.changelog'
>  
> -    def get_values(self, h, args, kw, headers):
> -        if args:
> -            revid = h.fix_revid(args[0])
> -        else:
> -            revid = None
> -
> -        filter_file_id = kw.get('filter_file_id', None)
> -        query = kw.get('q', None)
> -        start_revid = h.fix_revid(kw.get('start_revid', None))
> +    def get_values(self, h, revid, path, kwargs, headers):
> +
> +        filter_file_id = kwargs.get('filter_file_id', None)
> +        query = kwargs.get('q', None)
> +        start_revid = h.fix_revid(kwargs.get('start_revid', None))
>          orig_start_revid = start_revid
>          pagesize = 20#int(config.get('pagesize', '20'))
>          search_failed = False
>  
> +        if filter_file_id is None and path is not None:
> +            filter_file_id = h.get_file_id(revid, path)
> +
>          try:
>              revid, start_revid, revid_list = h.get_view(
>                  revid, start_revid, filter_file_id, query)
> -            util.set_context(kw)
> +            util.set_context(kwargs)
>  
>              if (query is not None) and (len(revid_list) == 0):
>                  search_failed = True

Same h arg here.

> @@ -74,18 +73,6 @@ class ChangeLogUI(TemplatedBranchView):
>          # add parent & merge-point branch-nick info, in case it's useful
>          h.get_branch_nicks(changes)
>  
> -        # does every change on this page have the same committer?  if so,
> -        # tell the template to show committer info in the "details block"
> -        # instead of on each line.
> -        all_same_author = True
> -
> -        if changes:
> -            author = changes[0].author
> -            for e in changes[1:]:
> -                if e.author != author:
> -                    all_same_author = False
> -                    break
> -
>          # Directory Breadcrumbs
>          directory_breadcrumbs = (
>              util.directory_breadcrumbs(
> @@ -105,7 +92,6 @@ class ChangeLogUI(TemplatedBranchView):
>              'viewing_from': (orig_start_revid is not None) and (orig_start_revid != h.last_revid),
>              'query': query,
>              'search_failed': search_failed,
> -            'all_same_author': all_same_author,
>              'url': self._branch.context_url,
>              'directory_breadcrumbs': directory_breadcrumbs,
>          }
> 
> === modified file 'loggerhead/controllers/directory_ui.py'
> --- loggerhead/controllers/directory_ui.py	2008-09-30 02:57:08 +0000
> +++ loggerhead/controllers/directory_ui.py	2008-10-25 18:13:27 +0000
> @@ -57,7 +57,7 @@ class DirectoryUI(TemplatedBranchView):
>          self._static_url_base = static_url_base
>          self.log = logging.getLogger('')
>  
> -    def get_values(self, h, args, kwargs, response):
> +    def get_values(self, h, revid, path, kwargs, response):
>          listing = [d for d in os.listdir(self._path)
>                     if not d.startswith('.')
>                     and os.path.isdir(os.path.join(self._path, d))]
> 
> === modified file 'loggerhead/controllers/inventory_ui.py'
> --- loggerhead/controllers/inventory_ui.py	2008-10-01 22:23:24 +0000
> +++ loggerhead/controllers/inventory_ui.py	2008-10-25 18:13:27 +0000
> @@ -1,4 +1,5 @@
>  #
> +# Copyright (C) 2008  Canonical Ltd.
>  # Copyright (C) 2006  Robey Pointer <robey@lag.net>
>  # Copyright (C) 2006  Goffredo Baroncelli <kreijack@inwind.it>
>  #
> @@ -19,6 +20,7 @@
>  
>  import logging
>  import posixpath
> +import urllib
>  
>  from paste.httpexceptions import HTTPServerError
>  
> @@ -31,9 +33,10 @@ from loggerhead.controllers import Templ
>  log = logging.getLogger("loggerhead.controllers")
>  
>  def dirname(path):
> -    while path.endswith('/'):
> -        path = path[:-1]
> -    path = posixpath.dirname(path)
> +    if path is not None:
> +        while path.endswith('/'):
> +            path = path[:-1]
> +        path = urllib.quote(posixpath.dirname(path))
>      return path
>  
>  
> @@ -41,14 +44,42 @@ class InventoryUI(TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.inventory'
>  
> -    def get_values(self, h, args, kw, headers):
> -        if len(args) > 0:
> -            revid = h.fix_revid(args[0])
> -        else:
> -            revid = h.last_revid
> +    def get_values(self, h, revid, path, kwargs, headers):
> +
> +        try:
> +            inv = h.get_inventory(revid)
> +        except:
> +            self.log.exception('Exception fetching changes')
> +            raise HTTPServerError('Could not fetch changes')
> +
> +        file_id = kwargs.get('file_id', None)
> +        start_revid = kwargs.get('start_revid', None)
> +        sort_type = kwargs.get('sort', None)
>  
>          # no navbar for revisions
>          navigation = util.Container()
> +
> +        change = h.get_changes([ revid ])[0]
> +        # add parent & merge-point branch-nick info, in case it's useful
> +        h.get_branch_nicks([ change ])
> +
> +        if path is not None:
> +            if not path.startswith('/'):
> +                path = '/' + path
> +            file_id = h.get_file_id(revid, path)
> +        else:
> +            path = inv.id2path(file_id)
> +        
> +        if file_id is None:
> +            file_id = inv.root.file_id
> +
> +
> +        idpath = inv.get_idpath(file_id)
> +        if len(idpath) > 1:
> +            updir = dirname(path)[1:]
> +        else:
> +            updir = None
> +
>          # Directory Breadcrumbs
>          directory_breadcrumbs = util.directory_breadcrumbs(
>                  self._branch.friendly_name,

h var is here as well.

> @@ -62,27 +93,10 @@ class InventoryUI(TemplatedBranchView):
>                  self.log.exception('Exception fetching changes')
>                  raise HTTPServerError('Could not fetch changes')
>  
> -            file_id = kw.get('file_id', inv.root.file_id)
> -            start_revid = kw.get('start_revid', None)
> -            sort_type = kw.get('sort', None)
> -
>              change = h.get_changes([ revid ])[0]
>              # add parent & merge-point branch-nick info, in case it's useful
>              h.get_branch_nicks([ change ])
>  
> -            path = inv.id2path(file_id)
> -            if not path.startswith('/'):
> -                path = '/' + path
> -            idpath = inv.get_idpath(file_id)
> -            if len(idpath) > 1:
> -                updir = dirname(path)
> -                updir_file_id = idpath[-2]
> -            else:
> -                updir = None
> -                updir_file_id = None
> -            if updir == '/':
> -                updir_file_id = None
> -
>              # Create breadcrumb trail for the path within the branch
>              branch_breadcrumbs = util.branch_breadcrumbs(path, inv, 'files')
>              filelist = h.get_filelist(inv, file_id, sort_type)
> @@ -95,7 +109,6 @@ class InventoryUI(TemplatedBranchView):
>              path = "/"
>              idpath = None
>              updir = None
> -            updir_file_id = None
>              branch_breadcrumbs = []
>              filelist = []
>  
> @@ -107,7 +120,6 @@ class InventoryUI(TemplatedBranchView):
>              'file_id': file_id,
>              'path': path,
>              'updir': updir,
> -            'updir_file_id': updir_file_id,
>              'filelist': filelist,
>              'history': h,
>              'posixpath': posixpath,
> 
> === modified file 'loggerhead/controllers/revision_ui.py'
> --- loggerhead/controllers/revision_ui.py	2008-08-16 21:53:52 +0000
> +++ loggerhead/controllers/revision_ui.py	2008-10-25 18:13:27 +0000
> @@ -30,21 +30,17 @@ class RevisionUI(TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.revision'
>  
> -    def get_values(self, h, args, kw, headers):
> +    def get_values(self, h, revid, path, kwargs, headers):
>  
> -        if len(args) > 0:
> -            revid = h.fix_revid(args[0])
> -        else:
> -            revid = None
> -
> -        filter_file_id = kw.get('filter_file_id', None)
> -        start_revid = h.fix_revid(kw.get('start_revid', None))
> -        query = kw.get('q', None)
> -        remember = h.fix_revid(kw.get('remember', None))
> -        compare_revid = h.fix_revid(kw.get('compare_revid', None))
> +        filter_file_id = kwargs.get('filter_file_id', None)
> +        start_revid = h.fix_revid(kwargs.get('start_revid', None))
> +        query = kwargs.get('q', None)
> +        remember = h.fix_revid(kwargs.get('remember', None))
> +        compare_revid = h.fix_revid(kwargs.get('compare_revid', None))
>  
>          try:
> -            revid, start_revid, revid_list = h.get_view(revid, start_revid, filter_file_id, query)
> +            revid, start_revid, revid_list = h.get_view(revid, start_revid, 
> +                                                        filter_file_id, query)
>          except:
>              self.log.exception('Exception fetching changes')
>              raise HTTPServerError('Could not fetch changes')

80 char line breaks here as well.

> @@ -68,7 +64,8 @@ class RevisionUI(TemplatedBranchView):
>                  line_count += len(chunk.diff)
>  
>          # let's make side-by-side diff be the default
> -        side_by_side = not kw.get('unified', False)
> +        # FIXME: not currently in use. Should be
> +        side_by_side = not kwargs.get('unified', False)
>          if side_by_side:
>              h.add_side_by_side([ change ])
>  
> 
> === modified file 'loggerhead/controllers/search_ui.py'
> --- loggerhead/controllers/search_ui.py	2008-09-09 22:42:27 +0000
> +++ loggerhead/controllers/search_ui.py	2008-10-25 18:13:27 +0000
> @@ -28,7 +28,7 @@ class SearchUI(TemplatedBranchView):
>  
>      template_path = 'loggerhead.templates.search'
>  
> -    def get_values(self, h, args, kwargs, response):
> +    def get_values(self, h, revid, path, kwargs, response):
>          """
>          Default method called from the search box as /search URL
>  
>

h arg...

> === modified file 'loggerhead/history.py'
> --- loggerhead/history.py	2008-09-27 05:13:24 +0000
> +++ loggerhead/history.py	2008-10-25 18:13:27 +0000
> @@ -35,6 +35,7 @@ import re
>  import textwrap
>  import threading
>  import time
> +import urllib
>  from StringIO import StringIO
>  
>  from loggerhead import search
> @@ -796,12 +797,15 @@ class History (object):
>              pathname = filename
>              if entry.kind == 'directory':
>                  pathname += '/'
> -
> +            if path == '':
> +                absolutepath = pathname
> +            else:
> +            	absolutepath = urllib.quote(path + '/' + pathname)
>              revid = entry.revision
>

There is a tab at absolutepath, can we convert to a space.

>              file = util.Container(
>                  filename=filename, executable=entry.executable, kind=entry.kind,
> -                pathname=pathname, file_id=entry.file_id, size=entry.text_size,
> +                pathname=pathname, absolutepath=absolutepath, file_id=entry.file_id, size=entry.text_size,
>                  revid=revid, change=change_dict[revid])
>              file_list.append(file)
>  
> 
> === modified file 'loggerhead/templates/inventory.pt'
> --- loggerhead/templates/inventory.pt	2008-10-01 22:15:10 +0000
> +++ loggerhead/templates/inventory.pt	2008-10-25 18:13:27 +0000
> @@ -53,9 +53,9 @@
>                <th class="expandcell"></th>
>              </tr>
>  
> -            <tr class="blueRow0" tal:condition="python:updir">
> +            <tr class="blueRow0" tal:condition="python:updir is not None">
>                  <td class="summcell" colspan="6">
> -                    <a tal:attributes="href python:url(['/files', revid], file_id=updir_file_id)" class="link">
> +                    <a tal:attributes="href python:url(['/files', change.revno, updir])" class="link">
>                          <img tal:attributes="src python:branch.static_url('/static/images/ico_folder_up.gif')" />..</a>
>                  </td>
>              </tr>
> @@ -63,12 +63,12 @@
>              
>              <tal:block repeat="file filelist">
>              <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='directory'">
> -                <td class="autcell"><a tal:attributes="href python:url(['/files', change.revno], file_id=file.file_id)">
> +                <td class="autcell"><a tal:attributes="href python:url(['/files', change.revno], file.absolutepath)">
>                          <img tal:attributes="src python:branch.static_url('/static/images/ico_folder.gif');
>                                               title string:Go to ${file/filename}" />
>                      </a>
>  
> -                 <a tal:attributes="href python:url(['/files', change.revno], file_id=file.file_id)" 
> +                 <a tal:attributes="href python:url(['/files', change.revno, file.absolutepath])" 
>                                          tal:content="file/filename" class="link"></a>
>                   </td>
>                   <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
> @@ -87,11 +87,11 @@
>  
>              
>              <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='symlink'">
> -                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)">
> +                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])">
>                          <img tal:attributes="src python:branch.static_url('/static/images/ico_flecha.gif')" alt="Symlink" />
>                      </a>
>  
> -                 <a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)" 
> +                 <a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])" 
>                                          tal:content="file/filename" class="link"></a>
>                   </td>
>                   <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
> @@ -111,7 +111,7 @@
>  
>              
>              <tr tal:attributes="class string:blueRow${file/parity}" tal:condition="python:file.kind=='file'">
> -                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)">
> +                <td class="autcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])">
>                          <img tal:attributes="src python:branch.static_url('/static/images/ico_file.gif');
>                                               title string:Annotate ${file/filename}"
>                               tal:condition="python:file.executable is False" />
> @@ -121,7 +121,7 @@
>                               tal:condition="python:file.executable is True" alt="File" />
>                      </a>
>  
> -              <a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id)"
> +              <a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath])"
>                                       tal:content="file/filename" class="link"></a></td>
>                <td class="date"><a tal:attributes="href python:url(['/revision', file.change.revno]);
>                                                    title string:Show revision ${file/change/revno}"
> @@ -129,7 +129,7 @@
>                </td>
>                <td class="date" tal:content="python:util.date_time(file.change.date)"></td>
>                <td class="timedate2" tal:content="python:util.human_size(file.size)"></td>
> -              <td class="expcell"><a tal:attributes="href python:url(['/annotate', change.revno], file_id=file.file_id);
> +              <td class="expcell"><a tal:attributes="href python:url(['/annotate', change.revno, file.absolutepath]);
>                                                       title string:Annotate ${file/filename}">
>                                        <img tal:attributes="src python:branch.static_url('/static/images/ico_planilla.gif')" alt="Diff" />
>                                       </a>
>

review: Needs Fixing

lp:~beuno/loggerhead/abstract_paths updated on 2008-10-25