Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread Junio C Hamano
David Aguilar  writes:

> On Fri, Jan 25, 2013 at 2:38 AM, John Keeping  wrote:
>> On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote:
>>> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty
>>> list "vim" as being an unavailable tool.  This is because they attempt
>>> to find a tool named according to the mergetool scriptlet instead of the 
>>> Git-
>>> recognized tool name.
>>
>> Actually, after my patches both git-difftool and git-mergetool get this
>> right since list_merge_tool_candidates lists vimdiff and gvimdiff.
>>
>>> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
>>> scriptlet.  This required git-mergetool--lib to special-case it when
>>> setting up the tool.
>>>
>>> Remove the exception for "vim" and allow the scriptlets to be found
>>> naturally by using symlinks to a single "vimdiff" scriptlet.
>>
>> I wonder if it would be better to make these single-line scripts instead
>> of symlinks:
>>
>> . "$MERGE_TOOLS_DIR"/vimdiff
>>
>> where we make git-mergetool--lib.sh export:
>>
>> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools
>
> That sounds like the way to go.

Yup, I'll expect a reroll of this one and possibly the next one (I
haven't read yet).

1-5/7 looked all very sensible.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread Sebastian Schuberth
On Fri, Jan 25, 2013 at 11:34 AM, Sebastian Schuberth
 wrote:

>> I thought Git did something sensible there like create a normal file?
>
> It does not. Also see my answer over here:
> http://stackoverflow.com/questions/11412028/git-not-storing-symlink-as-a-symlink-on-windows/11412341#11412341

This one might be the even more appropriate answer:
http://stackoverflow.com/questions/11662868/what-happens-when-i-clone-a-repository-with-symlinks-on-windows/11664406#11664406

-- 
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread David Aguilar
On Fri, Jan 25, 2013 at 2:38 AM, John Keeping  wrote:
> On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote:
>> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty
>> list "vim" as being an unavailable tool.  This is because they attempt
>> to find a tool named according to the mergetool scriptlet instead of the Git-
>> recognized tool name.
>
> Actually, after my patches both git-difftool and git-mergetool get this
> right since list_merge_tool_candidates lists vimdiff and gvimdiff.
>
>> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
>> scriptlet.  This required git-mergetool--lib to special-case it when
>> setting up the tool.
>>
>> Remove the exception for "vim" and allow the scriptlets to be found
>> naturally by using symlinks to a single "vimdiff" scriptlet.
>
> I wonder if it would be better to make these single-line scripts instead
> of symlinks:
>
> . "$MERGE_TOOLS_DIR"/vimdiff
>
> where we make git-mergetool--lib.sh export:
>
> MERGE_TOOLS_DIR=$(git --exec-path)/mergetools

That sounds like the way to go.
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread John Keeping
On Fri, Jan 25, 2013 at 01:43:53AM -0800, David Aguilar wrote:
> "git difftool --tool-help" and "git mergetool --tool-help" incorreclty
> list "vim" as being an unavailable tool.  This is because they attempt
> to find a tool named according to the mergetool scriptlet instead of the Git-
> recognized tool name.

Actually, after my patches both git-difftool and git-mergetool get this
right since list_merge_tool_candidates lists vimdiff and gvimdiff.

> vimdiff, vimdiff2, gvimdiff, and gvimdiff2 are all provided by the "vim"
> scriptlet.  This required git-mergetool--lib to special-case it when
> setting up the tool.
> 
> Remove the exception for "vim" and allow the scriptlets to be found
> naturally by using symlinks to a single "vimdiff" scriptlet.

I wonder if it would be better to make these single-line scripts instead
of symlinks:

. "$MERGE_TOOLS_DIR"/vimdiff

where we make git-mergetool--lib.sh export:

MERGE_TOOLS_DIR=$(git --exec-path)/mergetools


John
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread David Aguilar
On Fri, Jan 25, 2013 at 2:23 AM, Sebastian Schuberth
 wrote:
> On 2013/01/25 10:43 , David Aguilar wrote:
>
>> Remove the exception for "vim" and allow the scriptlets to be found
>> naturally by using symlinks to a single "vimdiff" scriptlet.  This
>
>
> I guess that won't work on platforms where Git does not support symlinks,
> then, like Windows. But Windows has (g)vimdiff, so loosing these on that
> platform would not be so nice.

I thought Git did something sensible there like create a normal file?

If not then I was thinking we could add a provides_tools() function that
each scriptlet could supply.
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] mergetools: Fix difftool/mergetool --tool-help listing for vim

2013-01-25 Thread Sebastian Schuberth

On 2013/01/25 10:43 , David Aguilar wrote:


Remove the exception for "vim" and allow the scriptlets to be found
naturally by using symlinks to a single "vimdiff" scriptlet.  This


I guess that won't work on platforms where Git does not support 
symlinks, then, like Windows. But Windows has (g)vimdiff, so loosing 
these on that platform would not be so nice.


--
Sebastian Schuberth

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html