On Thu, Jan 24, 2013 at 11:55 AM, John Keeping <john@xxxxxxxxxxxxx> wrote:
> The "--tool-help" option to git-difftool currently displays incorrect
> output since it uses the names of the files in
> "$GIT_EXEC_PATH/mergetools/" rather than the list of command names in
> git-mergetool--lib.
>
> Fix this by simply delegating the "--tool-help" argument to the
> show_tool_help function in git-mergetool--lib.
Very nice.
One thought I had was that the unified show_tool_help should
probably check TOOL_MODE=diff and skip over the
!can_diff entries.
The current output of "git difftool --tool-help" before your
patches has the problem that it will list tools such as
"tortoisemerge" as "valid but not available" because it
does not differentiate between missing and !can_diff.
> diff --git a/git-difftool.perl b/git-difftool.perl
> index edd0493..0a90de4 100755
> --- a/git-difftool.perl
> +++ b/git-difftool.perl
> @@ -59,57 +59,16 @@ sub find_worktree
> return $worktree;
> }
>
> -sub filter_tool_scripts
> -{
> - my ($tools) = @_;
> - if (-d $_) {
> - if ($_ ne ".") {
> - # Ignore files in subdirectories
> - $File::Find::prune = 1;
> - }
> - } else {
> - if ((-f $_) && ($_ ne "defaults")) {
> - push(@$tools, $_);
> - }
> - }
> -}
> -
> sub print_tool_help
> {
> - my ($cmd, @found, @notfound, @tools);
> - my $gitpath = Git::exec_path();
> -
> - find(sub { filter_tool_scripts(\@tools) }, "$gitpath/mergetools");
> -
> - foreach my $tool (@tools) {
> - $cmd = "TOOL_MODE=diff";
> - $cmd .= ' && . "$(git --exec-path)/git-mergetool--lib"';
> - $cmd .= " && get_merge_tool_path $tool >/dev/null 2>&1";
> - $cmd .= " && can_diff >/dev/null 2>&1";
> - if (system('sh', '-c', $cmd) == 0) {
> - push(@found, $tool);
> - } else {
> - push(@notfound, $tool);
> - }
> - }
This is where we conflated can_diff with "is the tool available?".
The nice thing is that the old code did actually check can_diff,
but since it conflated these things it ended up putting them
in the @notfound list. It should ignore those completely, IMO.
That could be a nice follow-up patch. What do you think?
--
David
--
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