I have successfully tested on:
- Linux with Git v1.7.10-rc0 and 1
- Windows 7 with msysgit v1.7.9
I installed cygwin Git on Windows 7, but found that 'git difftool'
would not work, even before my patches were applied. It appears that
I need to learn more about cygwin before I can use it as a test
platform.
On Mon, Mar 19, 2012 at 5:00 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> * In [1/9], use of pass_through would mean 'git difftool' must be fed the
> options for difftool and then options meant for underlying diff
> machinery. Is this limitation still there? If so, isn't this a
> regression? Shouldn't it at least be advertised to the users a lot
> stronger in documentation?
Both the documentation and testing has shown that 'pass_through' works
as I expected. That is, options that difftool cares about are
extracted from @ARGV while all others are passed on to the underlying
'git diff' command. The order of the arguments/options does not
matter. We "use 5.008;" at the top of the script, so I believe this
change is correct and safe. That being said, I have a limited number
of computers/configurations to test on.
> * In [4/9], you remove the .exe extension when running git, which was
> there since the beginning of difftool 5c38ea3 (contrib: add 'git
> difftool' for launching common merge tools, 2009-01-16). I think it is
> safe but are Windows folks OK with this?
My testing on Windows with msysgit has shown that this change is safe.
Also, even though I had other problems when testing on cygwin (noted
at the top of the email), the 'difftool' script was able to
successfully execute 'git diff --raw -z' without appending '.exe'.
> * In [6/9], the exit code in the failure case seem to be modified from
> that of the underlying "git diff" to whatever croak gives us. Is this
> a regression, or nobody cares error status from difftool? I personally
> suspect it is the latter, but just double-checking if you have
> considered it.
In my testing, if "Git::command_noisy(('diff', @ARGV))" fails, the
exit code from 'git diff' is passed back to the terminal. So not only
is the 'git diff' exit code echoed to the terminal, but '$?' also
contains the correct exit code.
> * In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't
> seem to be any inclusion of git-sh-setup in this script, and the patch
> does not have any effort to prepend $prefix to paths relative to $cwd.
> What good does the variable do here?
This was an oversight. It was copied over from a previous
implementation that still used ". git-sh-setup".
So the only outstanding changes that I am planning are:
1) Remove SUBDIRECTORY_OK
2) Add tests for the new '--dir-diff' option
Would it be better to resend the complete patches series with these
changes or just add new patches to the end?
Please let me know if I missed anything else.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html