[MERGE] Refactor diffing

Aaron Bentley aaron.bentley at utoronto.ca
Fri Nov 23 04:32:54 GMT 2007


-----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.

> 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 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.

>     """Provides textual representations of the difference between two
> trees.

I was being vague because of the possible non-textual diffing.  But that
would probably involve subclassing DiffTree anyhow, so okay.  The rest
looks nice.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHRlf20F+nu1YWqI0RAuBEAJ9vyQJ3a5ScwlQJGPrUvnhdReLXUQCZASZg
+pRwLlQDvz8Hw7MveryAkhQ=
=Jjqh
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: refactor-diff-4.patch
Type: text/x-diff
Size: 41646 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20071122/132fd944/attachment-0001.bin 


More information about the bazaar mailing list