[ANN][PLUGIN] extending diff with graphical tools

[John Arbash Meinel]

Stephen Ward wrote:
> Hi,
> 
> I've updated the difftools plugin to handle non-recursive tools by iterating 
> over all the modified files (note: like 'bzr diff', it is skipping the  
> added/deleted files).
> 
> It also uses the working tree whenever possible now (instead of temp dirs), so 
> that edits can be saved most of the time.  The main exceptions are diffs 
> between two old revisions, and diffs between two branches (where the 2nd 
> branch is the "new" revision); these still use temp dirs for both trees.
> 
> I've also added an initializer for 'mgdiff'.
> 
> Give it a try, and let me know if it isn't working the way you'd expect.
> 
> -- Steve
> 

Oh, one other thing I think would be nice would be to do is be creative
about the naming of the temporary directory.

If you look at my version of the vimdiff plugin, I use the temporary
name of:
/tmp/filenameaoeuaoeu.10.tmp.py
aoeuaoeu => the temporary stuff that NamedTemporaryFile adds to make
sure it is unique
filename => the original name of the file
10 => the revno of the file
.py => the original suffix of the file

I probably would change it to:

/tmp/filename.aoeuaoeu.10.tmp.py

Having the extension last is important so vim (and other editors)
automatically recognize the filetype. Having the revno helps a lot when
comparing 2 old revisions, and the filename makes it clear you are
comparing the same file.
By the way, bzr vimdiff won't work on Windows, because
NamedTemporaryFile cannot be accessed by another process. Windows
creates it such that it is *only* accessible by the current process. I'm
not sure if there is some way to "chmod" the file to get it readable by
another process, but by default it is not.

Also vimdiff does chmod(0444) so it is clear that the file is readonly.

Because of how you are doing things differently, I might suggest that
your "mkdtemp" tries specially to insert the revno (if available).

Oh, and at least for vimdiff & gvimdiff it is better to put the file you
want to modify as the first argument, since that way the file the cursor
starts in is the one you are changing.

I noticed that the extmerge plugin uses this notation:

# The same syntax as in mergetools below, can be used in external_merge
# in bazaar.conf
# %b = base  (foo.BASE)
# %t = this  (foo.THIS)
# %o = other (foo.OTHER)
# %r = resolved file (aka output file) (foo)

mergetools = [
        'kdiff3 --output %r %b %t %o',
        'xxdiff -m -O -M %r %t %b %o',
        'opendiff %t %o -ancestor %b -merge %r']

Which allows you to specify what order files should occur. This probably
doesn't work as well for your stuff, since most diff routines want the
new file second, and it isn't necessarily worth the extra effort.


And as a final small thing, if you want to get the code into the core of
bzr, PEP8 says to use an indent level of 4 spaces, not 2. But for a
plugin, you are allowed to do whatever you want. (I would also move more
of the code into diffutils.py rather than being in __init__.py).

Overall, I think your plugin has very nice functionality. It is coming
along quite nicely. I haven't really done any sort of code audit, not
knowing if you want that level of feedback, or if you just want UI level
feedback.

Thank you for all of your work,
John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060501/d2ffa7d8/attachment.pgp