[MERGE] Refactor diffing

Robert Collins robertc at robertcollins.net
Fri Nov 23 04:49:15 GMT 2007


On Thu, 2007-11-22 at 23:32 -0500, Aaron Bentley wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Robert Collins wrote:
> > On Thu, 2007-11-22 at 14:45 -0500, Aaron Bentley wrote:
> 
> >>> I think the extra_differs parameter is ok, though if those differs
> >>> will need the same state - old_tree, new_tree, to_file,
> >>> path_encoding - then I would recommend that it be
> >>> extra_differ_factories, and you do:
> >> No, they won't necessarily be constructed with the same state.  They may
> >> take more state.
> >
> > Its quite easy to create a factory that binds additional state to a
> > factory.
> 
> Sure, but the the extra state would typically be derived from
> commandline parameters, so it wouldn't be available at registration time.

Right, thats why the extra_factories in my example, so that you can bind
the extra state and still pass it in as a factory. IOW I agree that you
need an 'extra_FOO' parameter, just disagree about it being instances vs
factories.

> > A few comments
> > 
> > Differ seems strange to me. We don't have e.g. TreeTransformer for
> > example.
> 
> But we do have Merger.  The reason I didn't name TreeTransform as an
> actor, is because I think of it primarily as holding a description of
> changes to make.
> 
> I don't like Diff by itself, because it's not clear to me that it's the
> verb, not the noun.  But perhaps VerbObjecting the names would work,
> like DiffTree, DiffSymlink, etc?

I like DiffTree etc.

> > I think it would be a bit easier to understand with a noun as
> > the class name, and more consistent with our other code.
> 
> Well, it's really not *a* diff.  It's something that performs the
> operation of diffing.  It doesn't represent, say, a unified diff.
> Naming it that way would lend to confusion, I think.
> 
> > 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'.
> 
> I can live with DiffPath.

Cool.

-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/e7bae18e/attachment.pgp 


More information about the bazaar mailing list