Re: [PATCH v6 2/2] mergetools: add winmerge as a builtin tool
[just wrapping up the unaswered questions in this thread]
On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote:
Quoting David Aguilar dav...@gmail.com:
+translate_merge_tool_path() {
+# Use WinMergeU.exe if it exists in $PATH
+if type -p WinMergeU.exe /dev/null 21
+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.
+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
--
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 v6 2/2] mergetools: add winmerge as a builtin tool
David Aguilar dav...@gmail.com writes: [just wrapping up the unaswered questions in this thread] ... On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote: Thanks for clarifications. I think all is good now? -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
On Fri, May 22, 2015 at 01:05:28PM -0700, Junio C Hamano wrote: David Aguilar dav...@gmail.com writes: [just wrapping up the unaswered questions in this thread] ... On Wed, May 20, 2015 at 01:09:29PM +0200, SZEDER Gábor wrote: Thanks for clarifications. I think all is good now? Yes, I think so. I just looked at what you have queued in pu and it looks good. Thanks all, -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
Hi Junio, On 2015-05-20 23:00, Junio C Hamano wrote: Sebastian Schuberth sschube...@gmail.com writes: On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. I did add the sort (and -u) by intention, to ensure that C:\Program Files (which is what %PROGRAMFILES% expands to by default) comes before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)% expands to by default), so that programs of the OS-native bitness are preferred. Yuck. So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as if they are variables that can point at arbitrary places, they in reality don't? Otherwise %PROGRAMFILES% may point at D:\Program while %PROGRAMFILES(X86)% may piont at C:\X86 and the latter would sort before the former, which would defeat that logic. Well, you are correct, theoretically an administrator could set the registry values (which are the source of the environment variables in question) of the `ProgramFilesDir` key in both HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion and HKEY_LOCAL_MACHINE\Software\WOW64\Microsoft\Windows\CurrentVersion to wildly different locations as you outlined. However, it is not supported by Microsoft to change those locations via the registry: https://support.microsoft.com/en-us/kb/933700 Ciao, Dscho -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. Not that I personally care too deeply, as I would expect that it is likely any one of them found would just work fine ;-) + do + if test -n $directory test -x $directory/$winmerge_exe + then + printf '%s' $directory/$winmerge_exe + return + fi + done + + printf WinMergeU.exe +} -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
Sebastian Schuberth sschube...@gmail.com writes: On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. I did add the sort (and -u) by intention, to ensure that C:\Program Files (which is what %PROGRAMFILES% expands to by default) comes before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)% expands to by default), so that programs of the OS-native bitness are preferred. Yuck. So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as if they are variables that can point at arbitrary places, they in reality don't? Otherwise %PROGRAMFILES% may point at D:\Program while %PROGRAMFILES(X86)% may piont at C:\X86 and the latter would sort before the former, which would defeat that logic. But of course if I view this not as a logic but as a heuristics that happens to do the right thing in common environments, it is perfectly OK ;-). I've queued the patches as-is. 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 v6 2/2] mergetools: add winmerge as a builtin tool
On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote: David Aguilar dav...@gmail.com writes: + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' | + cut -d '=' -f 2- | sort -u) Is the final sort really desired? I am wondering if there are fixed precedence/preference order among variants of %PROGRAMFILES% environment variables that the users on the platform are expected to stick to, but the sort is sorting by the absolute pathnames of where these things are, which may not reflect that order. I did add the sort (and -u) by intention, to ensure that C:\Program Files (which is what %PROGRAMFILES% expands to by default) comes before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)% expands to by default), so that programs of the OS-native bitness are preferred. -- 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 v6 2/2] mergetools: add winmerge as a builtin tool
On Wed, May 20, 2015 at 11:00 PM, Junio C Hamano gits...@pobox.com wrote: Yuck. So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as if they are variables that can point at arbitrary places, they in reality don't? Otherwise %PROGRAMFILES% may point at D:\Program Correct. In the vast majority of WIndows installations these variables contain the default values. But of course if I view this not as a logic but as a heuristics that happens to do the right thing in common environments, it is perfectly OK ;-). Exactly a heuristic is what it's supposed to be :-) -- 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
