[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