[MERGE] Refactor diffing

Robert Collins robertc at robertcollins.net
Thu Nov 22 22:02:27 GMT 2007


On Thu, 2007-11-22 at 14:45 -0500, Aaron Bentley wrote:

A few comments

Differ seems strange to me. We don't have e.g. TreeTransformer for
example. I think it would be a bit easier to understand with a noun as
the class name, and more consistent with our other code.

The tests look good.

I think FileDiffer should be PathDiff, because the interface handles
diffing anything in a tree - file or directory. Or perhaps EntryDiff, or
something less likely to be confused with 'regular files'.


+class TreeDiffer(object):
+    """Object for comparing the contents of two trees"""

How about this as a docstring (using your class names, see above for my
rename preference).
    """Provides textual representations of the difference between two
trees.

    A TreeDiffer examines two tree and where a fileid has altered
    between them generates a textual representation of the difference.
    TreeDiffer uses a sequence of PathDiff objects which are each 
    given the opportunity to handle a given altered fileid. The list
    of PathDiff objects can be extended globally by appending to 
    TreeDiffer._diff_factories, or for a specific diff operation by
    supplying the extra_differ option to the appropriate method.
    """

Basically a bit of prose to make it obvious to folk encountering this
class what it's all about, hopefully without lots of duplication
(because its about intent and structure).

It might also be nice to reference TreeDiffer and FileDiffer as key
things to look at in the module docstring.

Martin may chime in here and say 'what about the developers guide'. I
think though that this stuff is really quite specific to this module,
and as long as the developers guide says 'text representations of diffs
are done by bzrlib.diff' then the loop is closed for finding out about
this stuff (without having to bootstrap all the way from 'bzr' ->
'bzr.commands' etc.)

-Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20071123/4785b3ad/attachment-0001.pgp 


More information about the bazaar mailing list