Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
- From
David Aguilar <davvid@gmail.com>
- Date
- May 22, 2015, 19:58 UTC
- Message-ID
- <20150522195802.GA26066@gmail.com>
- In-Reply-To
- <20150520130929.Horde.vYwOuIDRpi6hr15rOUbW1w7@webmail.informatik.kit.edu>
[just wrapping up the unaswered questions in this thread]
On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote:
Show 16 quoted lines
>
> Quoting David Aguilar <davvid@gmail.com>:
>
> >+translate_merge_tool_path() {
> >+ # Use WinMergeU.exe if it exists in $PATH
> >+ if type -p WinMergeU.exe >/dev/null 2>&1
> >+ then
> >+ printf WinMergeU.exe
> >+ return
> >+ fi
> >+
> >+ # Look for WinMergeU.exe in the typical locations
> >+ winmerge_exe="WinMerge/WinMergeU.exe"
>
> This variable is not used elsewhere, right? Then you might want to
> mark it as local to make this clear."local" is a bash-ism, otherwise that'd be a good idea.
Show 17 quoted lines
> >+ for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | > >+ cut -d '=' -f 2- | sort -u) > >+ do > >+ if test -n "$directory" && test -x "$directory/$winmerge_exe" > >+ then > >+ printf '%s' "$directory/$winmerge_exe" > >+ return > >+ fi > >+ done > >+ > >+ printf WinMergeU.exe > > Please pardon my ignorance and curiosity, but what is the purpose of > this last printf? > It outputs the same as in the case when winmerge is in $PATH at the > beginning of the function. However, if we reach this printf, then > winmerge is not in $PATH, so what will be executed?
This function maps what we call the tool (winmerge) to the actual executable. That last printf provides the following behavior:
$ git difftool -t winmerge HEAD~ Viewing (1/1): 'mergetools/winmerge' Launch 'winmerge' [Y/n]: The diff tool winmerge is not available as 'WinMergeU.exe' fatal: external diff died, stopping at mergetools/winmerge
It ensures that the user sees 'WinMergeU.exe' in the error message. That way the user can resolve the problem by e.g. adjusting their $PATH, or realizing that they don't have WinMergeU.exe installed.
-- David