[PATCH] send external diff to a file (bug 4047)

[John Arbash Meinel]

James Henstridge wrote:
> This patch is based on a patch I did a while ago to allow redirecting
> diff output to a file when using an external diff process.  Currently
> the external_diff() function contains the following check which
> prevents this:
> 
> def external_diff(old_filename, oldlines, new_filename, newlines, to_file,
>                  diff_opts):
>    """Display a diff by calling out to the external diff program."""
>    import sys
> 
>    if to_file != sys.stdout:
>        raise NotImplementedError("sorry, can't send external diff
> other than to stdout yet",
>                                  to_file)
>    ...
> 
> 
> The patch alters external_diff() to use the subprocess module to
> execute the external command rather than os.spawnv() which makes it
> possible to direct the output to a file descriptor other than stdout. I
> also added a simple test for the external_diff() function as Martin
> requested in the bug report.
> 
> I haven't added support for directing output to file like objects
> without an associated file descriptor (e.g. StringIO), so left a check
> for that condition in the code.
> 
> The changes are in my bzr branch here (currently at revno 1695):
>    http://people.ubuntu.com/~jamesh/bzr/bzr.smallfixes
> 
> James.
> 

And sys.stdout isn't guaranteed to be a real file either, since it may
have been internally changed.
So this fix is a good thing in general.

> 
> ------------------------------------------------------------------------
> 
> === modified file 'a/bzrlib/diff.py'
> --- a/bzrlib/diff.py	
> +++ b/bzrlib/diff.py	
> @@ -74,17 +74,15 @@
>  def external_diff(old_filename, oldlines, new_filename, newlines, to_file,
>                    diff_opts):
>      """Display a diff by calling out to the external diff program."""
> -    import sys
> -    
> -    if to_file != sys.stdout:
> -        raise NotImplementedError("sorry, can't send external diff other than to stdout yet",
> -                                  to_file)
> -
> +    if not hasattr(to_file, 'fileno'):
> +        raise NotImplementedError("sorry, can't send external diff other "
> +                                  "than to a file descriptor", to_file)
> +    
>      # make sure our own output is properly ordered before the diff
>      to_file.flush()
>  
>      from tempfile import NamedTemporaryFile
> -    import os
> +    import subprocess
>  

These imports should really happen at the module level, not inside the
diff function.

Otherwise it looks good.

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/20060504/814d7206/attachment.pgp