Ticket #199: diff_module_alpha2.diff

htdocs/css/browser.css

@@ -45 +45 @@
 #dirlist td.name a, #dirlist td.rev a { border-bottom: none; display: block }
 #dirlist td.change * { font-size: 9px }
 
+/* Log */
+#diff { margin: 0; }
+
 /* Styles for the revision log table
    (extends the styles for "table.listing") */
 #chglist { margin-top: 0 }

trac/db_default.py

@@ -458 +458 @@
 
 default_components = ('trac.About', 'trac.attachment', 'trac.Browser',
                       'trac.Changeset', 'trac.Search', 'trac.Settings',
+                      'trac.Diff',
                       'trac.ticket.query', 'trac.ticket.report',
                       'trac.Roadmap',
                       'trac.ticket.web_ui', 'trac.Timeline', 'trac.wiki.web_ui',

trac/versioncontrol/svn_fs.py

@@ -27 +27 @@
 import os.path
 import time
 import weakref
+import posixpath
 
 from svn import fs, repos, core, delta
 
@@ -175 +176 @@
     def __del__(self):
         self.close()
 
+    def has_node(self, path, rev):
+        rev_root = fs.revision_root(self.fs_ptr, rev, self.pool)
+        node_type = fs.check_path(rev_root, path, self.pool)
+        return node_type in _kindmap
+
     def normalize_path(self, path):
-        return path == '/' and path or path.strip('/')
+        return path == '/' and path or path and path.strip('/') or ''
 
     def normalize_rev(self, rev):
         try:
@@ -267 +273 @@
         rev = self.normalize_rev(rev)
         expect_deletion = False
         while rev:
-            rev_root = fs.revision_root(self.fs_ptr, rev, self.pool)
-            node_type = fs.check_path(rev_root, path, self.pool)
-            if node_type in _kindmap: # then path exists at that rev
+            if self.has_node(path, rev):
                 if expect_deletion:
                     # it was missing, now it's there again: rev+1 must be a delete
                     yield path, rev+1, Changeset.DELETE
@@ -294 +298 @@
                 expect_deletion = True
                 rev = self.previous_rev(rev)
 
+    def get_diffs(self, old_path, old_rev, new_path, new_rev, ignore_ancestry=1):
+        old_node = new_node = None
+        old_rev = self.normalize_rev(old_rev)
+        new_rev = self.normalize_rev(new_rev)
+        if self.has_node(old_path, old_rev):
+            old_node = self.get_node(old_path, old_rev)
+            old_path = old_node.created_path
+            old_rev = old_node.created_rev
+        if self.has_node(new_path, new_rev):
+            new_node = self.get_node(new_path, new_rev)
+            new_path = new_node.created_path
+            new_rev = new_node.created_rev
+        if not old_node and not new_node:
+            raise TracError, ('None of the diff arguments are valid: '
+                              'both %s at %s and %s at %s don\'t exist '
+                              'in the repository' % (old_path, old_rev,
+                                                     new_path, new_rev))
+        elif old_node and new_node:
+            if new_node.kind != old_node.kind:
+                raise TracError, ('Diff mismatch: Trying to diff a %s (%s at %s) '
+                                  'with a %s (%s at %s).' \
+                                  % (old_node.kind, old_path, old_rev,
+                                     new_node.kind, new_path, new_rev))
+        if new_node:
+            isdir = new_node.isdir
+        else:
+            isdir = old_node.isdir
+        if isdir:
+            old_base = old_path
+            new_base = new_path
+        else:
+            old_base = posixpath.split(old_path)[0]
+            new_base = posixpath.split(new_path)[0]
 
+        editor = DiffChangeEditor()
+        e_ptr, e_baton = delta.make_editor(editor, self.pool)
+        old_root = fs.revision_root(self.fs_ptr, old_rev, self.pool)
+        new_root = fs.revision_root(self.fs_ptr, new_rev, self.pool)
+        if isdir:
+            old_dir, old_entry = old_path, ''
+        else:
+            old_dir, old_entry = posixpath.split(old_path)
+        def authz_cb(root, path, pool): return 1
+        text_deltas = 0 # as this is currently re-done in Diff.py...
+        entry_props = 0 # ("... typically used only for working copy updates")
+        repos.svn_repos_dir_delta(old_root,
+                                  old_dir, old_entry,
+                                  new_root, new_path,
+                                  e_ptr, e_baton, authz_cb,
+                                  text_deltas,
+                                  isdir and 1 or 0,
+                                  entry_props,
+                                  ignore_ancestry,
+                                  self.pool)
+        for d in editor.deltas:
+            yield (posixpath.join(old_base,d[0]), posixpath.join(new_base,d[0]),
+                   d[1], d[2])
+
+
 class SubversionNode(Node):
 
     pool = property(fget=lambda self: self._pool(),
@@ -450 +512 @@
 
     def _get_prop(self, name):
         return fs.revision_prop(self.fs_ptr, self.rev, name, self.pool)
+
+
+#
+# Delta editor for diffs between arbitrary nodes (recycling my old code for #295 :) )
+#
+# Note 1: the 'copyfrom_path' and 'copyfrom_rev' information is not used
+#         because 'repos.svn_repos_dir_delta' *doesn't* provide it.
+#
+# Note 2: the 'dir_baton' is the path of the parent directory
+#
+
+class DiffChangeEditor(delta.Editor): 
+
+    def __init__(self):
+        self.deltas = []
+        self.skip_dir_prop_change = 0
+
+    def _norm(self, path):
+        """Path are normalized to __not__ have a leading slash"""
+        return path == '/' and path or path and path.strip('/') or ''
+    
+    # -- svn.delta.Editor callbacks
+
+    def open_root(self, base_revision, dir_pool):
+        return '/'
+
+    def add_directory(self, path, dir_baton, copyfrom_path, copyfrom_rev, dir_pool):
+        self.deltas.append((path, Node.DIRECTORY, Changeset.ADD))
+        # don't create an additional 'Changeset.EDIT' entry for this directory
+        # in case there's also a dir property change:
+        self.skip_dir_prop_change = 1 
+        return path
+
+    def open_directory(self, path, dir_baton, base_revision, dir_pool):
+        self.deltas.append((path, Node.DIRECTORY, Changeset.EDIT))
+        self.skip_dir_prop_change = 0
+        return path
+
+    def change_dir_prop(self, dir_baton, name, value, pool):
+        if self.skip_dir_prop_change:
+            return
+        self.deltas.append((dir_baton, Node.DIRECTORY, Changeset.EDIT))
+        self.skip_dir_prop_change = 1
+
+    def close_directory(self, dir_baton):
+        self.skip_dir_prop_change = 0
+
+    def delete_entry(self, path, revision, dir_baton, pool):
+        self.deltas.append((path, Node.FILE, Changeset.DELETE)) # should be Node.UNKNOWN
+
+    def add_file(self, path, dir_baton, copyfrom_path, copyfrom_revision, dir_pool):
+        self.deltas.append((self._norm(path), Node.FILE, Changeset.ADD))
+
+    def open_file(self, path, dir_baton, dummy_rev, file_pool):
+        self.deltas.append((self._norm(path), Node.FILE, Changeset.EDIT))
+

trac/versioncontrol/cache.py

@@ -88 +88 @@
     def get_node(self, path, rev=None):
         return self.repos.get_node(path, rev)
 
+    def has_node(self, path, rev):
+        return self.repos.has_node(path, rev)
+
     def get_oldest_rev(self):
         return self.repos.oldest_rev
 
@@ -112 +115 @@
     def normalize_rev(self, rev):
         return self.repos.normalize_rev(rev)
 
+    def get_diffs(self, old_path, old_rev, new_path, new_rev, ignore_ancestry=1):
+        return self.repos.get_diffs(old_path, old_rev, new_path, new_rev, ignore_ancestry)
 
+
 class CachedChangeset(Changeset):
 
     def __init__(self, rev, db, authz):

trac/versioncontrol/main.py

@@ -42 +42 @@
         """
         raise NotImplementedError
 
+    def has_node(self, path, rev):
+        """
+        Tell if there's a node at the specified (path,rev) combination.
+        """
+        raise NotImplementedError
+    
     def get_node(self, path, rev=None):
         """
         Retrieve a Node (directory or file) from the repository at the
@@ -114 +120 @@
         'None' is a valid revision value and represents the youngest revision.
         """
         return NotImplementedError
-        
 
+    def get_diffs(self, old_path, old_rev, new_path, new_rev, ignore_ancestry=1):
+        """
+        Generator that yields (old_path, new_path, kind, change) tuples
+        for each node change between the two arbitrary (path,rev) pairs.
+        """
+        raise NotImplementedError
+
+
 class Node(object):
     """
     Represents a directory or file in the repository.

trac/Diff.py

@@ -24 +24 @@
 from __future__ import generators
 import time
 import re
+import posixpath
 
 from trac import mimeview, perm, util
 from trac.core import *
-from trac.Timeline import ITimelineEventProvider
 from trac.versioncontrol import Changeset, Node
 from trac.versioncontrol.diff import get_diff_options, hdf_diff, unified_diff
 from trac.web.chrome import add_link, add_stylesheet
@@ -35 +35 @@
 from trac.wiki import wiki_to_html, wiki_to_oneliner
 
 
-class ChangesetModule(Component):
+class Diff(dict):
+    def __getattr__(self,str):
+        return self[str]
+    
 
-    implements(IRequestHandler, ITimelineEventProvider)
+class DiffModule(Component):
 
+    implements(IRequestHandler)
+
     # IRequestHandler methods
 
     def match_request(self, req):
-        match = re.match(r'/changeset/([0-9]+)$', req.path_info)
+        match = re.match(r'/diff(?:(/.*)|$)', req.path_info)
         if match:
-            req.args['rev'] = match.group(1)
+            req.args['path'] = match.group(1)
             return 1
 
     def process_request(self, req):
         req.perm.assert_permission(perm.CHANGESET_VIEW)
 
-        rev = req.args.get('rev')
+        path = req.args.get('path')
         repos = self.env.get_repository(req.authname)
+        path = repos.normalize_path(path)
+        rev = req.args.get('rev', repos.youngest_rev) # 'path history' mode
+        old = req.args.get('old')                     # 'arbitrary diff' mode
+        new = req.args.get('new')
+        old_path = req.args.get('old_path', path)
 
         diff_options = get_diff_options(req)
         if req.args.has_key('update'):
-            req.redirect(self.env.href.changeset(rev))
+            if old or new:
+                req.redirect(self.env.href.diff(path, new=new, old_path=old_path, old=old))
+            else:
+                req.redirect(self.env.href.diff(path, rev=rev))
 
-        chgset = repos.get_changeset(rev)
-        req.check_modified(chgset.date,
-                           diff_options[0] + ''.join(diff_options[1]))
+        if old or new:
+            chgset = None
+            if not new:
+                new = repos.next_rev(old) # FIXME: must lookup the next entry in node history
+            elif not old:
+                old = repos.previous_rev(new)
+            if not old_path:
+                old_path = path
+            diff = Diff(old_path=old_path, old_rev=old,
+                        new_path=path, new_rev=new)
+        else:
+            chgset = repos.get_changeset(rev)
+            diff = Diff(old_path=path, old_rev=repos.previous_rev(rev),
+                        new_path=path, new_rev=rev)
+        
+        # TODO:
+#         req.check_modified(chgset.date,
+#                            diff_options[0] + ''.join(diff_options[1]))
 
         format = req.args.get('format')
         if format == 'diff':
-            self._render_diff(req, repos, chgset, diff_options)
+            self._render_diff(req, repos, diff, chgset, diff_options)
             return
         elif format == 'zip':
-            self._render_zip(req, repos, chgset)
+            self._render_zip(req, repos, diff, chgset)
             return
 
-        self._render_html(req, repos, chgset, diff_options)
+        self._render_html(req, repos, diff, chgset, diff_options)
         add_link(req, 'alternate', '?format=diff', 'Unified Diff',
                  'text/plain', 'diff')
         add_link(req, 'alternate', '?format=zip', 'Zip Archive',
                  'application/zip', 'zip')
         add_stylesheet(req, 'changeset.css')
         add_stylesheet(req, 'diff.css')
-        return 'changeset.cs', None
+        return 'diff.cs', None
 
-    # ITimelineEventProvider methods
 
-    def get_timeline_filters(self, req):
-        if req.perm.has_permission(perm.CHANGESET_VIEW):
-            yield ('changeset', 'Repository checkins')
-
-    def get_timeline_events(self, req, start, stop, filters):
-        if 'changeset' in filters:
-            absurls = req.args.get('format') == 'rss' # Kludge
-            show_files = int(self.config.get('timeline',
-                                             'changeset_show_files'))
-            db = self.env.get_db_cnx()
-            repos = self.env.get_repository()
-            rev = repos.youngest_rev
-            while rev:
-                chgset = repos.get_changeset(rev)
-                if chgset.date < start:
-                    return
-                if chgset.date < stop:
-                    if absurls:
-                        href = self.env.abs_href.changeset(chgset.rev)
-                    else:
-                        href = self.env.href.changeset(chgset.rev)
-                    title = 'Changeset <em>[%s]</em> by %s' % (
-                            util.escape(chgset.rev), util.escape(chgset.author))
-                    message = wiki_to_oneliner(util.shorten_line(chgset.message or '--'),
-                                               self.env, db, absurls=absurls)
-                    if show_files:
-                        files = []
-                        for chg in chgset.get_changes():
-                            if show_files > 0 and len(files) >= show_files:
-                                files.append('...')
-                                break
-                            files.append('<span class="%s">%s</span>'
-                                         % (chg[2], util.escape(chg[0])))
-                        message = '<span class="changes">' + ', '.join(files) +\
-                                  '</span>: ' + message
-                    yield 'changeset', href, title, chgset.date, chgset.author,\
-                          message
-                rev = repos.previous_rev(rev)
-
     # Internal methods
 
-    def _render_html(self, req, repos, chgset, diff_options):
+    def _render_html(self, req, repos, diff, chgset, diff_options):
         """HTML version"""
-        req.hdf['title'] = '[%s]' % chgset.rev
-        req.hdf['changeset'] = {
-            'revision': chgset.rev,
-            'time': time.strftime('%c', time.localtime(chgset.date)),
-            'author': util.escape(chgset.author or 'anonymous'),
-            'message': wiki_to_html(chgset.message or '--', self.env, req,
-                                    escape_newlines=True)
-        }
+        req.hdf['diff'] = diff
+        req.hdf['diff.href'] = {
+            'new_rev': self.env.href.changeset(diff.new_rev),
+            'old_rev': self.env.href.changeset(diff.old_rev),
+            'new_path': self.env.href.browser(diff.new_path, rev=diff.new_rev),
+            'old_path': self.env.href.changeset(diff.old_path, rev=diff.old_rev)
+            }
+        if chgset: # 'path history' mode
+            req.hdf['title'] = 'Changes for %s at Revision %s' % (diff.new_path, chgset.rev)
+            req.hdf['changeset'] = {
+                'revision': chgset.rev,
+                'time': time.strftime('%c', time.localtime(chgset.date)),
+                'author': util.escape(chgset.author or 'anonymous'),
+                'message': wiki_to_html(chgset.message or '--', self.env, req,
+                                        escape_newlines=True)
+                }
 
-        oldest_rev = repos.oldest_rev
-        if chgset.rev != oldest_rev:
-            add_link(req, 'first', self.env.href.changeset(oldest_rev),
-                     'Changeset %s' % oldest_rev)
-            previous_rev = repos.previous_rev(chgset.rev)
-            add_link(req, 'prev', self.env.href.changeset(previous_rev),
-                     'Changeset %s' % previous_rev)
-        youngest_rev = repos.youngest_rev
-        if str(chgset.rev) != str(youngest_rev):
-            next_rev = repos.next_rev(chgset.rev)
-            add_link(req, 'next', self.env.href.changeset(next_rev),
-                     'Changeset %s' % next_rev)
-            add_link(req, 'last', self.env.href.changeset(youngest_rev),
-                     'Changeset %s' % youngest_rev)
+            oldest_rev = repos.oldest_rev
+            if chgset.rev != oldest_rev:
+                add_link(req, 'first', self.env.href.diff(diff.old_path, rev=oldest_rev),
+                         'Changeset %s' % oldest_rev) # FIXME (use the history)
+                previous_rev = repos.previous_rev(chgset.rev)
+                add_link(req, 'prev', self.env.href.diff(diff.old_path, rev=previous_rev),
+                         'Changeset %s' % previous_rev)
+            youngest_rev = repos.youngest_rev
+            if str(chgset.rev) != str(youngest_rev):
+                next_rev = repos.next_rev(chgset.rev)
+                add_link(req, 'next', self.env.href.diff(diff.new_path, rev=next_rev),
+                         'Changeset %s' % next_rev)
+                add_link(req, 'last', self.env.href.diff(diff.new_path, rev=youngest_rev),
+                         'Changeset %s' % youngest_rev)
+        elif diff.new_path == diff.old_path: # 'diff between 2 revisions' mode
+            req.hdf['title'] = 'Diff for %s between Revisions %s and %s' \
+                               % (diff.new_path, diff.old_rev, diff.new_rev)
+        else:                           # 'arbitrary diff' mode
+            req.hdf['title'] = 'Diff between %s at Revision %s and %s at Revision %s' \
+                               % (diff.old_path, diff.old_rev,
+                                  diff.new_path, diff.new_rev)
 
         edits = []
         idx = 0
-        for path, kind, change, base_path, base_rev in chgset.get_changes():
+        old_rev = diff.old_rev
+        new_rev = diff.new_rev
+        for old_path, new_path, kind, change in repos.get_diffs(**diff):
+            print 'delta %d: %s %s delta from %s@%s to %s@%s' \
+                  % (idx, change, kind, old_path, old_rev, new_path, new_rev)
             info = {'change': change}
-            if base_path:
-                info['path.old'] = base_path
-                info['rev.old'] = base_rev
-                info['browser_href.old'] = self.env.href.browser(base_path,
-                                                                 rev=base_rev)
-            if path:
-                info['path.new'] = path
-                info['rev.new'] = chgset.rev
-                info['browser_href.new'] = self.env.href.browser(path,
-                                                                 rev=chgset.rev)
+            if old_path:
+                info['path.old'] = old_path
+                info['rev.old'] = old_rev
+                info['browser_href.old'] = self.env.href.browser(old_path,
+                                                                 rev=old_rev)
+            if new_path:
+                info['path.new'] = new_path
+                info['rev.new'] = new_rev
+                info['browser_href.new'] = self.env.href.browser(new_path,
+                                                                 rev=new_rev)
             if change in (Changeset.COPY, Changeset.EDIT, Changeset.MOVE):
-                edits.append((idx, path, kind, base_path, base_rev))
-            req.hdf['changeset.changes.%d' % idx] = info
+                edits.append((idx, old_path, new_path, kind))
+            req.hdf['diff.changes.%d' % idx] = info
             idx += 1
-
-        for idx, path, kind, base_path, base_rev in edits:
-            old_node = repos.get_node(base_path or path, base_rev)
-            new_node = repos.get_node(path, chgset.rev)
-
+        
+        for idx, old_path, new_path, kind in edits:
+            old_node = repos.get_node(old_path, old_rev)
+            new_node = repos.get_node(new_path, new_rev)
+            
             # Property changes
             old_props = old_node.get_properties()
             new_props = new_node.get_properties()
@@ -183 +190 @@
                 for k,v in new_props.items():
                     if not k in old_props:
                         changed_props[k] = {'new': v}
-                req.hdf['changeset.changes.%d.props' % idx] = changed_props
+                req.hdf['diff.changes.%d.props' % idx] = changed_props
 
             if kind == Node.DIRECTORY:
                 continue
 
             # Content changes
             default_charset = self.config.get('trac', 'default_charset')
-            old_content = old_node.get_content().read()
+            old_content = old_node.get_content().read()            
             if mimeview.is_binary(old_content):
                 continue
             charset = mimeview.get_charset(old_node.content_type) or \
@@ -217 +224 @@
                                    ignore_blank_lines='-B' in diff_options[1],
                                    ignore_case='-i' in diff_options[1],
                                    ignore_space_changes='-b' in diff_options[1])
-                req.hdf['changeset.changes.%d.diff' % idx] = changes
+                req.hdf['diff.changes.%d.diff' % idx] = changes
 
-    def _render_diff(self, req, repos, chgset, diff_options):
+
+    def _render_diff(self, req, repos, chgset, diff_options): # TODO
         """Raw Unified Diff version"""
         req.send_response(200)
         req.send_header('Content-Type', 'text/plain;charset=utf-8')
@@ -284 +292 @@
                                          ignore_space_changes='-b' in diff_options[1]):
                     req.write(line + util.CRLF)
 
-    def _render_zip(self, req, repos, chgset):
+    def _render_zip(self, req, repos, chgset): # TODO
         """ZIP archive with all the added and/or modified files."""
         req.send_response(200)
         req.send_header('Content-Type', 'application/zip')

trac/Browser.py

@@ -118 +118 @@
             'props': dict([(util.escape(name), util.escape(value))
                            for name, value in node.get_properties().items()]),
             'href': self.env.href.browser(path,rev=rev or repos.youngest_rev),
+            'diff_href': self.env.href.diff(path,rev=rev or repos.youngest_rev),
             'log_href': self.env.href.log(path)
         }
 
@@ -268 +269 @@
         stop_rev = req.args.get('stop_rev')
         verbose = req.args.get('verbose')
         limit = int(req.args.get('limit') or 100)
+        diff = req.args.get('diff')
+        old = req.args.get('old')
+        new = req.args.get('new')
 
+        repos = self.env.get_repository(req.authname)
+        normpath = repos.normalize_path(path)
+        rev = str(repos.normalize_rev(rev))
+        old = old or str(repos.previous_rev(rev))
+        new = new or rev
+
         req.hdf['title'] = path + ' (log)'
         req.hdf['log'] = {
             'path': path,
@@ -276 +286 @@
             'verbose': verbose,
             'stop_rev': stop_rev,
             'browser_href': self.env.href.browser(path, rev=rev),
-            'log_href': self.env.href.log(path, rev=rev)
+            'log_href': self.env.href.log(path, rev=rev),
+            'select_diff_href': self.env.href.log(path, rev=rev, diff=1),
+            'diff': diff,
+            'diff_href': self.env.href.diff(path, old=old, new=new),
+            'old': old,
+            'new': new
         }
 
         path_links = _get_path_links(self.env.href, path, rev)
@@ -284 +299 @@
         if path_links:
             add_link(req, 'up', path_links[-1]['href'], 'Parent directory')
 
-        repos = self.env.get_repository(req.authname)
-        normpath = repos.normalize_path(path)
-        rev = str(repos.normalize_rev(rev))
-
         # 'node' or 'path' history: use get_node()/get_history() or get_path_history()
         if mode != 'path_history':
             try:

templates/log.cs

@@ -2 +2 @@
 <?cs include "macros.cs"?>
 
 <div id="ctxtnav" class="nav">
- <ul>
-  <li class="last"><a href="<?cs
-    var:log.browser_href ?>">View Latest Revision</a></li><?cs
+ <ul><?cs 
+  if:!log.diff ?>
+   <li class="first">
+    <a href="<?cs var:log.select_diff_href ?>">Select for Diff</a>
+   </li><?cs
+  /if ?>
+  <li class="last">
+   <a href="<?cs var:log.browser_href ?>">View Latest Revision</a>
+  </li><?cs
   if:len(chrome.links.prev) ?>
    <li class="first<?cs if:!len(chrome.links.next) ?> last<?cs /if ?>">
     &larr; <a href="<?cs var:chrome.links.prev.0.href ?>" title="<?cs
@@ -74 +80 @@
    </dl>
   </div>
  </div>
- <table id="chglist" class="listing">
-  <thead>
-   <tr>
+ <table id="chglist" class="listing"><?cs
+   if:log.diff ?>
+    <form action="<?cs var:log.diff_href ?>" method="get"><?cs
+   /if ?>
+  <thead><?cs
+   if:log.diff ?>
+    <tr>
+     <th colspan="2">
+      <div class="buttons">
+       <input type="submit" id="diff" value="Diff" 
+        title="Diff from Old Revision to New Revision (select them below)" />
+      </div>
+     </th>
+    </tr><?cs
+   /if ?>
+   <tr><?cs 
+    if:log.diff ?>
+     <th>Old</th>
+     <th>New</th><?cs
+    /if ?>
     <th class="change"></th>
     <th class="data">Date</th>
     <th class="rev">Rev</th>