Code review comment for lp://staging/~ace17/bzr/fixDiff4

Revision history for this message
Sebastien Alaiwan (ace17) wrote :

Hi Richard,

Using the value of the "--using" option as a command line part would indeed do the trick.
In fact, this is what I first tried (bzr diff --using "gvim -d -f -n").
However, I never got it to work. Here's what I get on my machine:

ace@ANTEC:~/projects/perdr2$ which ls
/bin/ls

ace@ANTEC:~/projects/perdr2$ bzr diff --using '/bin/ls -l'
=== modified file 'Makefile'
bzr: ERROR: /bin/ls -l could not be found on this machine

ace@ANTEC:~/projects/perdr2$ bzr --version
Bazaar (bzr) 2.7.0dev1

It seems the code responsible for launching the "--using" diff tool interprets the whole string as a program name, bypassing the system shell - which I believe is a good thing, for lots a reasons, including portability and security ones.

So, I'm OK with the following: passing "--diff-options" to external diff, or only to the "--using" diff tool if there's one.

Seems like you got a different result than mine on your computer; how did you do that?! :-)

BTW, is there a reason to have three ways of invoking diff tools? Could the current "external diff" (GNU diff) be an extra DiffTool, or does it play a special role?

« Back to merge proposal