{"thread":{"id":"39318","subject":"[PATCH v2] mergetools: add winmerge as a builtin tool","startedAt":"2015-05-13T02:00:07Z","lastAt":"2015-05-20T04:10:23Z","messageCount":14,"participants":["David Aguilar","Johannes Schindelin","Sebastian Schuberth","SZEDER Gábor"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"261112","messageId":"1431482407-63642-1-git-send-email-davvid@gmail.com","threadId":"39318","inReplyTo":null,"subject":"[PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-05-13T02:00:07Z","receivedAt":"2015-05-13T02:00:07Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Add a winmerge scriptlet with the commands described in [1] so\nthat users can use winmerge without needing to perform any\nadditional configuration.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/268631\n\nHelped-by: Philip Oakley \nHelped-by: Johannes Schindelin \nSigned-off-by: David Aguilar \n---\n\nChanges since v1: we now honor the $ProgramW6432 environment variable.\n\n mergetools/winmerge | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n create mode 100644 mergetools/winmerge\n\ndiff --git a/mergetools/winmerge b/mergetools/winmerge\nnew file mode 100644\nindex 0000000..db0b060\n--- /dev/null\n+++ b/mergetools/winmerge\n@@ -0,0 +1,40 @@\n+diff_cmd () {\n+\t\"$merge_tool_path\" -u -e \"$LOCAL\" \"$REMOTE\"\n+\treturn 0\n+}\n+\n+merge_cmd () {\n+\t# mergetool.winmerge.trustExitCode is implicitly false.\n+\t# touch $BACKUP so that we can check_unchanged.\n+\ttouch \"$BACKUP\"\n+\t\"$merge_tool_path\" -u -e -dl Local -dr Remote \\\n+\t\t\"$LOCAL\" \"$REMOTE\" \"$MERGED\"\n+\tcheck_unchanged\n+}\n+\n+translate_merge_tool_path() {\n+\t# Use WinMergeU.exe if it exists in $PATH\n+\tif type WinMergeU.exe >/dev/null 2>&1\n+\tthen\n+\t\techo WinMergeU.exe\n+\t\treturn\n+\tfi\n+\n+\t# Look for WinMergeU.exe in the typical locations\n+\twinmerge_exe=\"WinMerge/WinMergeU.exe\"\n+\tif test -n \"$ProgramW6432\" && test -x \"$ProgramW6432/$winmerge_exe\"\n+\tthen\n+\t\tprintf '%s' \"$ProgramW6432/$winmerge_exe\"\n+\telif test -n \"$PROGRAMFILES\" && test -x \"$PROGRAMFILES/$winmerge_exe\"\n+\tthen\n+\t\tprintf '%s' \"$PROGRAMFILES/$winmerge_exe\"\n+\telif test -x \"/c/Program Files (x86)/$winmerge_exe\"\n+\tthen\n+\t\tprintf '%s' \"/c/Program Files (x86)/$winmerge_exe\"\n+\telif test -x \"/c/Program Files/$winmerge_exe\"\n+\tthen\n+\t\tprintf '%s' \"/c/Program Files/$winmerge_exe\"\n+\telse\n+\t\techo WinMergeU.exe\n+\tfi\n+}\n-- \n1.8.5.3\n"},{"id":"261140","messageId":"96de4a8c540c041733fd47a4cb1af782@www.dscho.org","threadId":"39318","inReplyTo":"1431482407-63642-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-13T08:10:24Z","receivedAt":"2015-05-13T08:10:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi David,\n\nOn 2015-05-13 04:00, David Aguilar wrote:\n\n> +\tif test -n \"$ProgramW6432\" && test -x \"$ProgramW6432/$winmerge_exe\"\n> +\tthen\n> +\t\tprintf '%s' \"$ProgramW6432/$winmerge_exe\"\n> +\telif test -n \"$PROGRAMFILES\" && test -x \"$PROGRAMFILES/$winmerge_exe\"\n> +\tthen\n> +\t\tprintf '%s' \"$PROGRAMFILES/$winmerge_exe\"\n\nI am terribly sorry that I confused you... What I *meant* was to test in the following order: $PROGRAMFILES, $ProgramW6432, /c/Program Files, /c/Program Files (x86).\n\nWhile I am already nitpicking, I would also like to throw out the idea to write that in a loop instead (a little bit more DRY):\n\nfor directory in \"$PROGRAMFILES\" \"$ProgramW6432\" '/c/Program Files' '/c/Program Files (x86)'\ndo\n test -n \"$directory\" &&\n test -x \"$directory/$winmerge_exe\" &&\n echo \"$directory/$winmerge_exe\" &&\n break\ndone\n\nCiao,\nDscho\n"},{"id":"261176","messageId":"55534F4E.60402@gmail.com","threadId":"39318","inReplyTo":"1431482407-63642-1-git-send-email-davvid@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-13T13:19:10Z","receivedAt":"2015-05-13T13:19:10Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 13.05.2015 04:00, David Aguilar wrote:\n\n> +\tif test -n \"$ProgramW6432\" && test -x \"$ProgramW6432/$winmerge_exe\"\n> +\tthen\n> +\t\tprintf '%s' \"$ProgramW6432/$winmerge_exe\"\n\nI don't think it makes sense to check \"$ProgramW6432\". The content of \nthat variable depends on the bitness of the process requesting the \nenvironment. Just checking \"$PROGRAMFILES\" and \"$PROGRAMFILES(X86)\" \nshould be sufficient and more clear.\n\nAlso, note that you should use all upper case names when referring to \nWindows environment variables. While it's true that on plain Windows \nenvironment variable names are case-insensitive, MSYS1/2 converts all \nvariable names to upper case and *is* case sensitive. I.e. while \"echo \n$PROGRAMFILES\" works as expected from a Git Bash on Windows, \"echo \n$ProgramFiles\" results in an empty string.\n\n > +\telif test -x \"/c/Program Files (x86)/$winmerge_exe\"\n > +\tthen\n > +\t\tprintf '%s' \"/c/Program Files (x86)/$winmerge_exe\"\n > +\telif test -x \"/c/Program Files/$winmerge_exe\"\n > +\tthen\n > +\t\tprintf '%s' \"/c/Program Files/$winmerge_exe\"\n\nI also think these fallbacks can simply go away. It is *very* unlikely \nthat \"$PROGRAMFILES\" / \"$PROGRAMFILES(X86)\" point to something else but \nWinMerge still is installed there.\n\n-- \nSebastian Schuberth\n"},{"id":"261182","messageId":"3d7e3b09b89c46c39befca7564f5c1d6@www.dscho.org","threadId":"39318","inReplyTo":"55534F4E.60402@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2015-05-13T15:22:23Z","receivedAt":"2015-05-13T15:22:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn 2015-05-13 15:19, Sebastian Schuberth wrote:\n> On 13.05.2015 04:00, David Aguilar wrote:\n> \n>> +\tif test -n \"$ProgramW6432\" && test -x \"$ProgramW6432/$winmerge_exe\"\n>> +\tthen\n>> +\t\tprintf '%s' \"$ProgramW6432/$winmerge_exe\"\n> \n> I don't think it makes sense to check \"$ProgramW6432\". The content of\n> that variable depends on the bitness of the process requesting the\n> environment. Just checking \"$PROGRAMFILES\" and \"$PROGRAMFILES(X86)\"\n> should be sufficient and more clear.\n\nIn my tests, \"$PROGRAMFILES(X86)\" did *not* work:\n\n $ echo \"$PROGRAMFILES(X86)\"\n bash: syntax error near unexpected token `('\n\n> Also, note that you should use all upper case names when referring to\n> Windows environment variables. While it's true that on plain Windows\n> environment variable names are case-insensitive, MSYS1/2 converts all\n> variable names to upper case and *is* case sensitive. I.e. while \"echo\n> $PROGRAMFILES\" works as expected from a Git Bash on Windows, \"echo\n> $ProgramFiles\" results in an empty string.\n\nExactly. In my tests, \"$ProgramW6432\" worked, while \"$PROGRAMW6432\" did not.\n\nFWIW I think that the idea to test for a WinMerge executable of another bitness makes sense, because we can execute an executable of another bitness (unlike a .dll of another bitness).\n\nCiao,\nJohannes\n"},{"id":"261184","messageId":"CAHGBnuPDSdCyVNM+Gagang1Cf9yw0Tyios45i6pnZSSzaJJC2Q@mail.gmail.com","threadId":"39318","inReplyTo":"3d7e3b09b89c46c39befca7564f5c1d6@www.dscho.org","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-13T15:33:53Z","receivedAt":"2015-05-13T15:33:53Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Wed, May 13, 2015 at 5:22 PM, Johannes Schindelin\n wrote:\n\n> In my tests, \"$PROGRAMFILES(X86)\" did *not* work:\n>\n> $ echo \"$PROGRAMFILES(X86)\"\n> bash: syntax error near unexpected token `('\n\nInteresting. In both MSYS1/2 Git Bashes I get on Windows 7 64-bit:\n\n$ echo \"$PROGRAMFILES(X86)\"\nC:\\Program Files (x86)(X86)\n\nSo it seems to resolve only the $PROGRAMFILES part and appending the\nliteral \"(X86)\". Not sure how to tell Bash that \"(X86)\" is part of the\nvariable name.\n\n> Exactly. In my tests, \"$ProgramW6432\" worked, while \"$PROGRAMW6432\" did not.\n\nVery odd indeed that for me it's the exact opposite.\n\n> FWIW I think that the idea to test for a WinMerge executable of another bitness makes sense, because we can execute an executable of another bitness (unlike a .dll of another bitness).\n\nI agree it makes sense to check for both 64-bit and 32-bit WinMerge\ninstallations. But IMO it does not make sense to mix in the bitness of\nthe process requesting the environment as part of running the\nmergetools/winmerge script, which is what we do by querying\n\"$ProgramW6432\" / \"$PROGRAMW6432\" .\n\n-- \nSebastian Schuberth\n"},{"id":"261199","messageId":"5553AD43.9010807@gmail.com","threadId":"39318","inReplyTo":"CAHGBnuPDSdCyVNM+Gagang1Cf9yw0Tyios45i6pnZSSzaJJC2Q@mail.gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-13T20:00:03Z","receivedAt":"2015-05-13T20:00:03Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 13.05.2015 17:33, Sebastian Schuberth wrote:\n\n>> In my tests, \"$PROGRAMFILES(X86)\" did *not* work:\n>>\n>> $ echo \"$PROGRAMFILES(X86)\"\n>> bash: syntax error near unexpected token `('\n> \n> Interesting. In both MSYS1/2 Git Bashes I get on Windows 7 64-bit:\n> \n> $ echo \"$PROGRAMFILES(X86)\"\n> C:\\Program Files (x86)(X86)\n> \n> So it seems to resolve only the $PROGRAMFILES part and appending the\n> literal \"(X86)\". Not sure how to tell Bash that \"(X86)\" is part of the\n> variable name.\n> \n>> Exactly. In my tests, \"$ProgramW6432\" worked, while \"$PROGRAMW6432\" did not.\n> \n> Very odd indeed that for me it's the exact opposite.\n\nSo how about something like this which hopefully covers all cases (including case-sensitivity issues regarding environment variable names and the problem of querying a variable that contains parentheses as part of its name):\n\nfor directory in \"$(env | sed -nr 's/^PROGRAM(FILES(\\(X86\\))?|W6432)=//pI')\"\ndo\n test -n \"$directory\" &&\n test -x \"$directory/$winmerge_exe\" &&\n echo \"$directory/$winmerge_exe\" &&\n break\ndone\n\nsed's \"I\" seems to be a GNU extension that's not available with OS X' BSD sed. This shouldn't be an issue as WinMerge is Windows only anyway. If it still turns out to be an issue we probably should come up with an equivalent Perl expression.\n\n-- \nSebastian Schuberth\n"},{"id":"261202","messageId":"5553B5BF.7040408@gmail.com","threadId":"39318","inReplyTo":"5553AD43.9010807@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-13T20:36:15Z","receivedAt":"2015-05-13T20:36:15Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 13.05.2015 22:00, Sebastian Schuberth wrote:\n\n> So how about something like this which hopefully covers all cases (including case-sensitivity issues regarding environment variable names and the problem of querying a variable that contains parentheses as part of its name):\n> \n> for directory in \"$(env | sed -nr 's/^PROGRAM(FILES(\\(X86\\))?|W6432)=//pI')\"\n> do\n> test -n \"$directory\" &&\n> test -x \"$directory/$winmerge_exe\" &&\n> echo \"$directory/$winmerge_exe\" &&\n> break\n> done\n> \n> sed's \"I\" seems to be a GNU extension that's not available with OS X' BSD sed. This shouldn't be an issue as WinMerge is Windows only anyway. If it still turns out to be an issue we probably should come up with an equivalent Perl expression.\n\nOr even better, as that does not rely on GNU sed, sorts matches so that \"C:\\Program Files\" for sure comes before \"C:\\Program Files (x86)\" and removes any duplicates:\n\nfor directory in \"$(env | grep -E '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut -d '=' -f 2 | sort -u)\"\ndo\n test -n \"$directory\" &&\n test -x \"$directory/$winmerge_exe\" &&\n echo \"$directory/$winmerge_exe\" &&\n break\ndone\n\n-- \nSebastian Schuberth\n"},{"id":"261203","messageId":"5553B61D.7050204@gmail.com","threadId":"39318","inReplyTo":"5553B5BF.7040408@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-13T20:37:49Z","receivedAt":"2015-05-13T20:37:49Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On 13.05.2015 22:36, Sebastian Schuberth wrote:\n\n> for directory in \"$(env | grep -E '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut -d '=' -f 2 | sort -u)\"\n\nSorry, that should have been \"grep -Ei\" in there.\n\n-- \nSebastian Schuberth\n"},{"id":"261296","messageId":"1431638653-9356-1-git-send-email-szeder@ira.uka.de","threadId":"39318","inReplyTo":"CAHGBnuPDSdCyVNM+Gagang1Cf9yw0Tyios45i6pnZSSzaJJC2Q@mail.gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-14T21:24:13Z","receivedAt":"2015-05-14T21:24:13Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\n> On Wed, May 13, 2015 at 5:22 PM, Johannes Schindelin gmx.de> wrote:\n> \n> > In my tests, \"$PROGRAMFILES(X86)\" did *not* work:\n> >\n> > $ echo \"$PROGRAMFILES(X86)\"\n> > bash: syntax error near unexpected token `('\n> \n> Interesting. In both MSYS1/2 Git Bashes I get on Windows 7 64-bit:\n> \n> $ echo \"$PROGRAMFILES(X86)\"\n> C:\\Program Files (x86)(X86)\n> \n> So it seems to resolve only the $PROGRAMFILES part and appending the\n> literal \"(X86)\". Not sure how to tell Bash that \"(X86)\" is part of the\n> variable name.\n\nIt would be ${PROGRAMFILES(X86)}, but POSIX says that variable names can\nonly contain alphanumeric characters and underscores, and Bash adheres\nwhen it complains about it in Johannes' example above. Not sure whether\nthere is a clever escaping that could make it work, I couldn't find any.\n\n\nG�bor\n"},{"id":"261299","messageId":"1431640110-8484-1-git-send-email-szeder@ira.uka.de","threadId":"39318","inReplyTo":"5553B61D.7050204@gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-14T21:48:30Z","receivedAt":"2015-05-14T21:48:30Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> for directory in \"$(env | grep -E '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut -d '=' -f 2 | sort -u)\"\n> do\n> test -n \"$directory\" &&\n> test -x \"$directory/$winmerge_exe\" &&\n> echo \"$directory/$winmerge_exe\" &&\n> break\n> done\n\nThis ain't gonna work, because the output of the pipe won't be split\nbecause of the quotes around the command substitution, so $directory will\nhold everything. Simply loosing those quotes is not good either, because\nthe output will be split on the spaces as well, leading to 'C:\\Program',\n'Files' and '(x86)', unless the space is already removed from IFS. IIRC\nWindows doesn't allow newlines in pathnames, so IFS=$'\\n' and removing the\nquotes around the command substitution should do the trick.\n\nPathnames can contain '=' characters, so, while I think it's outrageous,\nthe paths in those variables can also contain an '=' or two. Well,\ntheoretically, at least, and in that case the 'cut -d= -f2' will cut off\nthe end of the pathname. I don't know whether this is something to be\nworth worrying about, but in that case it should be 'cut -d= -f2-'.\n\n\nG�bor\n"},{"id":"261320","messageId":"CAHGBnuNvTZ27CCquHHtC5bQFkG2k-+SDYw-6U2TRzzHyFiFwJw@mail.gmail.com","threadId":"39318","inReplyTo":"1431638653-9356-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-15T05:40:53Z","receivedAt":"2015-05-15T05:40:53Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, May 14, 2015 at 11:24 PM, SZEDER Gábor wrote:\n\n>> $ echo \"$PROGRAMFILES(X86)\"\n>> C:\\Program Files (x86)(X86)\n>>\n>> So it seems to resolve only the $PROGRAMFILES part and appending the\n>> literal \"(X86)\". Not sure how to tell Bash that \"(X86)\" is part of the\n>> variable name.\n>\n> It would be ${PROGRAMFILES(X86)}, but POSIX says that variable names can\n> only contain alphanumeric characters and underscores, and Bash adheres\n> when it complains about it in Johannes' example above. Not sure whether\n> there is a clever escaping that could make it work, I couldn't find any.\n\nRight, I did already try that curly-braces-style of quoting, and\nMSYS1/2 both give:\n\n$ echo ${PROGRAMFILES(X86)}\nsh.exe\": ${PROGRAMFILES(X86)}: bad substitution\n\nI couldn't find any other style of quoting that works, either.\n\n-- \nSebastian Schuberth\n"},{"id":"261321","messageId":"CAHGBnuN-sw8Q_boB2SSNKTQP6M4jifiketFWJ23bknAdCVw_Uw@mail.gmail.com","threadId":"39318","inReplyTo":"1431640110-8484-1-git-send-email-szeder@ira.uka.de","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"Sebastian Schuberth","fromEmail":"sschuberth@gmail.com","sentAt":"2015-05-15T05:46:23Z","receivedAt":"2015-05-15T05:46:23Z","isPatch":true,"sender":{"key":"sschuberth@gmail.com","avatar":"https://avatars.githubusercontent.com/u/349154?v=4"},"body":"On Thu, May 14, 2015 at 11:48 PM, SZEDER Gábor wrote:\n\n>> for directory in \"$(env | grep -E '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut -d '=' -f 2 | sort -u)\"\n>> do\n>> test -n \"$directory\" &&\n>> test -x \"$directory/$winmerge_exe\" &&\n>> echo \"$directory/$winmerge_exe\" &&\n>> break\n>> done\n>\n> This ain't gonna work, because the output of the pipe won't be split\n> because of the quotes around the command substitution, so $directory will\n> hold everything. Simply loosing those quotes is not good either, because\n\nThat's what I thought at first, too, but simply trying it out proved\nme wrong (again, in both MSYS1/2):\n\n$ for d in \"$(env | grep -Ei '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut\n-d '=' -f 2- | sort -u)\"; do echo \"$d\"; done\nC:\\Program Files\nC:\\Program Files (x86)\n\nNote that the path, although they are containing spaces, are on separate lines.\n\n-- \nSebastian Schuberth\n"},{"id":"261331","messageId":"20150515110117.Horde.HyhfKlrEun2r-3mHUG3GSA1@webmail.informatik.kit.edu","threadId":"39318","inReplyTo":"CAHGBnuN-sw8Q_boB2SSNKTQP6M4jifiketFWJ23bknAdCVw_Uw@mail.gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-15T09:01:17Z","receivedAt":"2015-05-15T09:01:17Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nQuoting Sebastian Schuberth :\n\n> On Thu, May 14, 2015 at 11:48 PM, SZEDER Gábor wrote:\n>\n>>> for directory in \"$(env | grep -E\n>>> '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut -d '=' -f 2 | sort -u)\"\n>>> do\n>>> test -n \"$directory\" &&\n>>> test -x \"$directory/$winmerge_exe\" &&\n>>> echo \"$directory/$winmerge_exe\" &&\n>>> break\n>>> done\n>>\n>> This ain't gonna work, because the output of the pipe won't be split\n>> because of the quotes around the command substitution, so $directory will\n>> hold everything. Simply loosing those quotes is not good either, because\n>\n> That's what I thought at first, too, but simply trying it out proved\n> me wrong (again, in both MSYS1/2):\n>\n> $ for d in \"$(env | grep -Ei '^PROGRAM(FILES(\\(X86\\))?|W6432)=' | cut\n> -d '=' -f 2- | sort -u)\"; do echo \"$d\"; done\n> C:\\Program Files\n> C:\\Program Files (x86)\n>\n> Note that the path, although they are containing spaces, are on\n> separate lines.\n\nYeah, exactly the behavior I described in the part you quoted.\nAlthough it looks good, it's not what you want.\nPutting 'echo \"** $d **\"' in the loop gives me:\n\n** C:\\Program Files\nC:\\Program Files (x86) **\n\n(1.9.5 MSysGit)\n\n\nGábor\n"},{"id":"261630","messageId":"20150520041017.GA91560@gmail.com","threadId":"39318","inReplyTo":"CAHGBnuNvTZ27CCquHHtC5bQFkG2k-+SDYw-6U2TRzzHyFiFwJw@mail.gmail.com","subject":"Re: [PATCH v2] mergetools: add winmerge as a builtin tool","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-05-20T04:10:23Z","receivedAt":"2015-05-20T04:10:23Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Fri, May 15, 2015 at 07:40:53AM +0200, Sebastian Schuberth wrote:\n> On Thu, May 14, 2015 at 11:24 PM, SZEDER Gábor wrote:\n> \n> >> $ echo \"$PROGRAMFILES(X86)\"\n> >> C:\\Program Files (x86)(X86)\n> >>\n> >> So it seems to resolve only the $PROGRAMFILES part and appending the\n> >> literal \"(X86)\". Not sure how to tell Bash that \"(X86)\" is part of the\n> >> variable name.\n> >\n> > It would be ${PROGRAMFILES(X86)}, but POSIX says that variable names can\n> > only contain alphanumeric characters and underscores, and Bash adheres\n> > when it complains about it in Johannes' example above. Not sure whether\n> > there is a clever escaping that could make it work, I couldn't find any.\n> \n> Right, I did already try that curly-braces-style of quoting, and\n> MSYS1/2 both give:\n> \n> $ echo ${PROGRAMFILES(X86)}\n> sh.exe\": ${PROGRAMFILES(X86)}: bad substitution\n> \n> I couldn't find any other style of quoting that works, either.\n\nHow about simply \"$PROGRAMFILES (x86)\"?\n\nLet the shell handle the programfiles part, and then it'll\nconcat the (x86). That's what's on the filesystem anyways, so\nit should DTRT. I'll send this in patch form shortly.\n-- \nDavid\n"}]}