git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] mergetools: add winmerge as a builtin tool

From
Sebastian Schuberth <sschuberth@gmail.com>
Date
May 13, 2015, 13:19 UTC
Message-ID
<55534F4E.60402@gmail.com>
In-Reply-To
<1431482407-63642-1-git-send-email-davvid@gmail.com>
On 13.05.2015 04:00, David Aguilar wrote:
> +	if test -n "$ProgramW6432" && test -x "$ProgramW6432/$winmerge_exe"
> +	then
> +		printf '%s' "$ProgramW6432/$winmerge_exe"

I don't think it makes sense to check "$ProgramW6432". The content of that variable depends on the bitness of the process requesting the environment. Just checking "$PROGRAMFILES" and "$PROGRAMFILES(X86)" should be sufficient and more clear.

Also, note that you should use all upper case names when referring to Windows environment variables. While it's true that on plain Windows environment variable names are case-insensitive, MSYS1/2 converts all variable names to upper case and *is* case sensitive. I.e. while "echo $PROGRAMFILES" works as expected from a Git Bash on Windows, "echo $ProgramFiles" results in an empty string.

 > +	elif test -x "/c/Program Files (x86)/$winmerge_exe"
 > +	then
 > +		printf '%s' "/c/Program Files (x86)/$winmerge_exe"
 > +	elif test -x "/c/Program Files/$winmerge_exe"
 > +	then
 > +		printf '%s' "/c/Program Files/$winmerge_exe"

I also think these fallbacks can simply go away. It is *very* unlikely that "$PROGRAMFILES" / "$PROGRAMFILES(X86)" point to something else but WinMerge still is installed there.

-- 
Sebastian Schuberth
Previous: Johannes SchindelinNext: Johannes Schindelin
Message 3 of 14 in “mergetools: add winmerge as a builtin tool”
  1. mergetools: add winmerge as a builtin toolDavid Aguilar, May 13, 2015
  2. Johannes SchindelinMay 13, 2015
  3. Sebastian SchuberthMay 13, 2015
  4. Johannes SchindelinMay 13, 2015
  5. Sebastian SchuberthMay 13, 2015
  6. Sebastian SchuberthMay 13, 2015
  7. Sebastian SchuberthMay 13, 2015
  8. Sebastian SchuberthMay 13, 2015
  9. SZEDER GáborMay 14, 2015
  10. Sebastian SchuberthMay 15, 2015
  11. SZEDER GáborMay 15, 2015
  12. SZEDER GáborMay 14, 2015
  13. Sebastian SchuberthMay 15, 2015
  14. David AguilarMay 20, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.