Re: [PATCH] use get_merge_tool_path() also in is_available() to honor settings

From: Adam Dinwoodie <adam@dinwoodie.org>
Date: Aug 31, 2021, 11:01 UTC
Message-ID: <CA+kUOamRLtsArhjMpo7A28=6vPabry8KsjM022gZ4nxDj0wU-Q@mail.gmail.com>
In-Reply-To: <CAJDDKr4rXEZDH4ZNU1ieUcfpOseikOVVfFQ=1tG9YNr2nibLCQ@mail.gmail.com>

On Mon, 30 Aug 2021 at 01:09, David Aguilar <davvid@gmail.com> wrote:

Show 9 quoted lines

> I'd like to extend 47eb4c6890 (mergetools/kdiff3: make kdiff3 work on
> Windows too, 2021-06-07)
> but in order to do so I need a reliable way to special-case Windows so
> that we only do the x -> x.exe translation there.
>
> What's the best way to detect Windows? t/test-lib.sh seems to detect
> Windows using "uname -s" and
> checking for matches against *MINGW* or *CYGWIN*. Is that reliable /
> recommended?

I don't think there's a single canonical way to do this, but that's certainly an effective way.

However I'd suggest not adding special-case code for Cygwin here, only for MinGW: the general design principles for Cygwin are such that *nix-native code should require minimal changes to work on Cygwin, and in particular the Cygwin compatibility layer should look after adding that extension, rather than requiring the application code to handle the special case. Both approaches would work, but I think it makes sense to follow the general Cygwin design principle here of not adding unnecessary special casing to handle it, and to leave it to the Cygwin compatibility layer instead.