Christian Boos wrote:
> Peter Dimov wrote:
>> Dear all,
>>
>> Attached is a patch based on trac-0.10.3 containing some refactoring to
>> the versioncontrol module (not the svn backend though).
>>
>
> Ouch, while it's OK to use 0.10-stable in order to show example code, we
> can't base further developments on that.
I finally got some spare time to look at that one again. I actually
wanted to port my previous modifications to the latest revision in trunk
but that required me to do some modifications in
LogModule.process_request(), which was about 140 lines and called for
refactoring!
Instead of writing big functions I normally like to write smaller, more
cohesive ones, which do only a single coherent part of the whole work.
The "big" function just handles the logic and delegates to the smaller
functions accordingly. IMHO this way the source code becomes much more
readable and easier to maintain. Thats definitely an approach that
could be applied at different places in trac (from what Ive seen,
LogModule and ChangesetModule at least).
Ive refactored LogModule.process_request() having the above described
considerations in mind. Thats just the first step and I guess it might
make sense to further split some of the new functions. Ive added an new
class LogRequest which handles a single request to the log module.
__init__() parses the inputs and the object stores the request state
between the invocations of the individual request processing steps. I
havent thoroughly tested the patch, which means I may have forgotten to
add a self. in front of a var somewhere.
I plan to do similar refactoring to ChangesetModule before proceeding
with the version control api refactoring. Actually I would further
refactor LogRequest and ChangesetRequest to use the "perfect" version
control api. I guess we would have some discussion about how perfect it
is before actually going ahead and changing it ;-)
Well, Id like to know what you think about all this before I do any
further work.
Best regards,
Peter
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/trac-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
Index: trac/versioncontrol/web_ui/log.py
===================================================================
--- trac/versioncontrol/web_ui/log.py (revision 4937)
+++ trac/versioncontrol/web_ui/log.py (working copy)
@@ -69,175 +69,13 @@
def process_request(self, req):
req.perm.assert_permission('LOG_VIEW')
- mode = req.args.get('mode', 'stop_on_copy')
- path = req.args.get('path', '/')
- rev = req.args.get('rev')
- stop_rev = req.args.get('stop_rev')
- revs = req.args.get('revs', rev)
- format = req.args.get('format')
- verbose = req.args.get('verbose')
- limit = int(req.args.get('limit') or self.default_log_limit)
+ request = LogRequest(self, req)
+ request.prepare_history()
+ request.extract_info()
+ request.extract_changes()
+ request.prepare_response()
+ return request.get_response()
- repos = self.env.get_repository(req.authname)
- normpath = repos.normalize_path(path)
- revranges = None
- rev = revs
- if revs:
- try:
- revranges = Ranges(revs)
- rev = revranges.b
- except ValueError:
- pass
- rev = unicode(repos.normalize_rev(rev))
- path_links = get_path_links(req.href, path, rev)
- if path_links:
- add_link(req, 'up', path_links[-1]['href'], 'Parent directory')
-
- # The `history()` method depends on the mode:
- # * for ''stop on copy'' and ''follow copies'', it's `Node.history()`
- # unless explicit ranges have been specified
- # * for ''show only add, delete'' we're using
- # `Repository.get_path_history()`
- if mode == 'path_history':
- rev = revranges.b
- def history(limit):
- for h in repos.get_path_history(path, rev, limit):
- yield h
- else:
- if not revranges or revranges.a == revranges.b:
- history = get_existing_node(req, repos, path, rev).get_history
- else:
- def history(limit):
- prevpath = path
- ranges = list(revranges.pairs)
- ranges.reverse()
- for (a,b) in ranges:
- while b >= a:
- rev = repos.normalize_rev(b)
- node = get_existing_node(req, repos, prevpath, rev)
- node_history = list(node.get_history(2))
- p, rev, chg = node_history[0]
- if rev < a:
- yield (p, rev, None) # separator
- break
- yield node_history[0]
- prevpath = node_history[-1][0] # follow copy
- b = rev-1
- if b < a and len(node_history) > 1:
- p, rev, chg = node_history[1]
- yield (p, rev, None)
-
- # -- retrieve history, asking for limit+1 results
- info = []
- depth = 1
- fix_deleted_rev = False
- previous_path = normpath
- for old_path, old_rev, old_chg in history(limit+1):
- if fix_deleted_rev:
- fix_deleted_rev['existing_rev'] = old_rev
- fix_deleted_rev = False
- if stop_rev and repos.rev_older_than(old_rev, stop_rev):
- break
- old_path = repos.normalize_path(old_path)
-
- item = {
- 'path': old_path, 'rev': old_rev, 'existing_rev': old_rev,
- 'change': old_chg, 'depth': depth,
- }
-
- if old_chg == Changeset.DELETE:
- fix_deleted_rev = item
- if not (mode == 'path_history' and old_chg == Changeset.EDIT):
- info.append(item)
- if old_path and old_path != previous_path \
- and not (mode == 'path_history' and old_path == normpath):
- depth += 1
- item['depth'] = depth
- item['copyfrom_path'] = old_path
- if mode == 'stop_on_copy':
- break
- if len(info) > limit: # we want limit+1 entries
- break
- previous_path = old_path
- if info == []:
- # FIXME: we should send a 404 error here
- raise TracError("The file or directory '%s' doesn't exist "
- "at revision %s or at any previous revision."
- % (path, rev), 'Nonexistent path')
-
- def make_log_href(path, **args):
- link_rev = rev
- if rev == str(repos.youngest_rev):
- link_rev = None
- params = {'rev': link_rev, 'mode': mode, 'limit': limit}
- params.update(args)
- if verbose:
- params['verbose'] = verbose
- return req.href.log(path, **params)
-
- if len(info) == limit+1: # limit+1 reached, there _might_ be some more
- next_rev = info[-1]['rev']
- next_path = info[-1]['path']
- add_link(req, 'next', make_log_href(next_path, rev=next_rev),
- 'Revision Log (restarting at %s, rev. %s)'
- % (next_path, next_rev))
- # only show fully 'limit' results, use `change == None` as a marker
- info[-1]['change'] = None
-
- revs = [i['rev'] for i in info]
- changes = get_changes(repos, revs)
- extra_changes = {}
- email_map = {}
-
- if format == 'rss':
- # Get the email addresses of all known users
- if Chrome(self.env).show_email_addresses:
- for username,name,email in self.env.get_known_users():
- if email:
- email_map[username] = email
- elif format == 'changelog':
- for rev in revs:
- changeset = changes[rev]
- cs = {}
- cs['message'] = wrap(changeset.message, 70,
- initial_indent='\t',
- subsequent_indent='\t')
- files = []
- actions = []
- for path, kind, chg, bpath, brev in changeset.get_changes():
- files.append(chg == Changeset.DELETE and bpath or path)
- actions.append(chg)
- cs['files'] = files
- cs['actions'] = actions
- extra_changes[rev] = cs
- data = {
- 'context': Context(self.env, req, 'source', path),
- 'path': path, 'rev': rev, 'stop_rev': stop_rev,
- 'mode': mode, 'verbose': verbose,
- 'path_links': path_links, 'limit' : limit,
- 'items': info, 'changes': changes,
- 'email_map': email_map, 'extra_changes': extra_changes,
- 'wiki_format_messages':
- self.config['changeset'].getbool('wiki_format_messages')
- }
-
- if req.args.get('format') == 'changelog':
- return 'revisionlog.txt', data, 'text/plain'
- elif req.args.get('format') == 'rss':
- return 'revisionlog.rss', data, 'application/rss+xml'
-
- add_stylesheet(req, 'common/css/diff.css')
- add_stylesheet(req, 'common/css/browser.css')
-
- rss_href = make_log_href(path, format='rss', stop_rev=stop_rev)
- add_link(req, 'alternate', rss_href, 'RSS Feed', 'application/rss+xml',
- 'rss')
- changelog_href = make_log_href(path, format='changelog',
- stop_rev=stop_rev)
- add_link(req, 'alternate', changelog_href, 'ChangeLog', 'text/plain')
-
- return 'revisionlog.html', data, None
-
# IWikiSyntaxProvider methods
REV_RANGE = r"(?:%s|%s)" % (Ranges.RE_STR, ChangesetModule.CHANGESET_ID)
@@ -304,4 +142,218 @@
ranges = ''.join([str(rev)+sep for rev, sep in zip(revs, seps)])
revranges = Ranges(ranges)
return str(revranges) or None
-
+
+
+class LogRequest:
+ """Completely handles a request to the LogModule."""
+
+ def __init__(self, module, req):
+ """Parses the data from req into member variables for use later."""
+
+ self.module = module
+ self.req = req
+ self.mode = req.args.get('mode', 'stop_on_copy')
+ self.path = req.args.get('path', '/')
+ self.rev = req.args.get('rev')
+ self.stop_rev = req.args.get('stop_rev')
+ self.revs = req.args.get('revs', self.rev)
+ self.format = req.args.get('format')
+ self.verbose = req.args.get('verbose')
+ self.limit = int(req.args.get('limit') or
self.module.default_log_limit)
+
+ self.repos = self.module.env.get_repository(req.authname)
+ self.normpath = self.repos.normalize_path(self.path)
+ self.revranges = None
+ self.rev = self.revs
+ if self.revs:
+ try:
+ self.revranges = Ranges(self.revs)
+ self.rev = self.revranges.b
+ except ValueError:
+ pass
+ self.rev = unicode(self.repos.normalize_rev(self.rev))
+
+ def _get_path_history(self, limit):
+ """A generator yielding the changes history of a path, i.e. possibly
+ different files or directories that have been moved/copied to this
+ location in different revisions."""
+
+ for h in self.repos.get_path_history(self.path, self.rev, limit):
+ yield h
+
+ def _get_node_history(self, limit):
+ """Returns a gererator yielding the changes to a node."""
+
+ return get_existing_node(self.req, self.repos, \
+ self.path, self.rev).get_history(limit)
+
+ def _get_node_history_ranges(self, limit):
+ """A generator yielding the changes to a node, restricted to certain
+ revisions. The revisions outside the given ranges are filtered."""
+
+ prevpath = self.path
+ ranges = list(self.revranges.pairs)
+ ranges.reverse()
+ for (a,b) in ranges:
+ while b >= a:
+ rev = repos.normalize_rev(b) # \todo
+ node = get_existing_node(self.req, self.repos, prevpath, rev)
+ node_history = list(node.get_history(2))
+ p, rev, chg = node_history[0]
+ if rev < a:
+ yield (p, rev, None) # separator
+ break
+ yield node_history[0]
+ prevpath = node_history[-1][0] # follow copy
+ b = rev-1
+ if b < a and len(node_history) > 1:
+ p, rev, chg = node_history[1]
+ yield (p, rev, None)
+
+ def prepare_history(self):
+ """Depending on the selected mode prepares and selects a generator
+ function, which will be used to extract the history in the next step.
+
+ * for ''stop on copy'' and ''follow copies'', it's
`Node.get_history()`
+ unless explicit ranges have been specified, in which case some of
the
+ entries are filtered.
+ * for ''show only add, delete'' we're using
+ `Repository.get_path_history()`"""
+
+ if self.mode == 'path_history':
+ self.rev = self.revranges.b
+ self.history = self._get_path_history
+ else:
+ if not self.revranges or self.revranges.a == self.revranges.b:
+ self.history = self._get_node_history
+ else:
+ self.history = self._get_node_history_ranges
+
+ def extract_info(self):
+ """Retrieves history and extracts info for at most limit+1 results"""
+
+ info = []
+ depth = 1
+ fix_deleted_rev = False
+ previous_path = self.normpath
+ for old_path, old_rev, old_chg in self.history(self.limit+1):
+ if fix_deleted_rev:
+ fix_deleted_rev['existing_rev'] = old_rev
+ fix_deleted_rev = False
+ if self.stop_rev and self.repos.rev_older_than(old_rev,
self.stop_rev):
+ break
+ old_path = self.repos.normalize_path(old_path)
+
+ item = {
+ 'path': old_path, 'rev': old_rev, 'existing_rev': old_rev,
+ 'change': old_chg, 'depth': depth,
+ }
+
+ if old_chg == Changeset.DELETE:
+ fix_deleted_rev = item
+ if not (self.mode == 'path_history' and old_chg == Changeset.EDIT):
+ info.append(item)
+ if old_path and old_path != previous_path \
+ and not (self.mode == 'path_history' and old_path == normpath):
+ depth += 1
+ item['depth'] = depth
+ item['copyfrom_path'] = old_path
+ if self.mode == 'stop_on_copy':
+ break
+ if len(info) > self.limit: # we want limit+1 entries
+ break
+ previous_path = old_path
+ if info == []:
+ # FIXME: we should send a 404 error here
+ raise TracError("The file or directory '%s' doesn't exist "
+ "at revision %s or at any previous revision."
+ % (path, rev), 'Nonexistent path')
+ else:
+ self.info = info
+
+ def extract_changes(self):
+ """Extract the required information for the revisions in the
history."""
+
+ self.revs = [i['rev'] for i in self.info]
+ self.changes = get_changes(self.repos, self.revs)
+
+ def prepare_response(self):
+ """Prepares the data to be returned as a response to this request."""
+
+ # \todo I actually wanted this get_path_links part to be in response()
+ # but path_links is needed for self.data (see below).
+ self.path_links = get_path_links(self.req.href, self.path, self.rev)
+ if self.path_links:
+ add_link(self.req, 'up', self.path_links[-1]['href'], 'Parent
directory')
+
+ extra_changes = {}
+ email_map = {}
+
+ if self.format == 'rss':
+ # Get the email addresses of all known users
+ if Chrome(self.module.env).show_email_addresses:
+ for username,name,email in self.module.env.get_known_users():
+ if email:
+ email_map[username] = email
+ elif self.format == 'changelog':
+ for rev in self.revs:
+ changeset = self.changes[rev]
+ cs = {}
+ cs['message'] = wrap(changeset.message, 70,
+ initial_indent='\t',
+ subsequent_indent='\t')
+ files = []
+ actions = []
+ for path, kind, chg, bpath, brev in changeset.get_changes():
+ files.append(chg == Changeset.DELETE and bpath or path)
+ actions.append(chg)
+ cs['files'] = files
+ cs['actions'] = actions
+ extra_changes[rev] = cs
+ self.data = {
+ 'context': Context(self.module.env, self.req, 'source', self.path),
+ 'path': self.path, 'rev': self.rev, 'stop_rev': self.stop_rev,
+ 'mode': self.mode, 'verbose': self.verbose,
+ 'path_links': self.path_links, 'limit' : self.limit,
+ 'items': self.info, 'changes': self.changes,
+ 'email_map': email_map, 'extra_changes': extra_changes,
+ 'wiki_format_messages':
+ self.module.config['changeset'].getbool('wiki_format_messages')
+ }
+
+ def _make_log_href(self, path, **args):
+ link_rev = self.rev
+ if self.rev == str(self.repos.youngest_rev):
+ link_rev = None
+ params = {'rev': link_rev, 'mode': self.mode, 'limit': self.limit}
+ params.update(args)
+ if self.verbose:
+ params['verbose'] = self.verbose
+ return self.req.href.log(path, **params)
+
+ def get_response(self):
+ if len(self.info) == self.limit+1: # limit+1 reached, there _might_ be
some more
+ next_rev = self.info[-1]['rev']
+ next_path = self.info[-1]['path']
+ add_link(self.req, 'next', self._make_log_href(next_path,
rev=next_rev),
+ 'Revision Log (restarting at %s, rev. %s)'
+ % (next_path, next_rev))
+ # only show fully 'limit' results, use `change == None` as a marker
+ self.info[-1]['change'] = None
+
+ if self.format == 'changelog':
+ return 'revisionlog.txt', self.data, 'text/plain'
+ elif self.format == 'rss':
+ return 'revisionlog.rss', self.data, 'application/rss+xml'
+
+ add_stylesheet(self.req, 'common/css/diff.css')
+ add_stylesheet(self.req, 'common/css/browser.css')
+
+ rss_href = self._make_log_href(self.path, format='rss',
stop_rev=self.stop_rev)
+ add_link(self.req, 'alternate', rss_href, 'RSS Feed',
'application/rss+xml',
+ 'rss')
+ changelog_href = self._make_log_href(self.path, format='changelog',
+ stop_rev=self.stop_rev)
+ add_link(self.req, 'alternate', changelog_href, 'ChangeLog',
'text/plain')
+
+ return 'revisionlog.html', self.data, None
