Support detecting the mintty terminal application on Cygwin / MSYS2 #127
Conversation
111617d to
095c0e1
Compare
|
|
||
| // https://github.com/mintty/mintty/issues/482 | ||
| val mintty = isMinttyTerminal() | ||
|
|
||
| val inputInteractive = interactive ?: (ij || mintty || stdinInteractive()) | ||
| val outputInteractive = interactive ?: (ij || mintty || stdoutInteractive()) | ||
|
|
There was a problem hiding this comment.
This will cause ANSI codes to be output even if stdout is redirected to a file, which we want to avoid. So let's remove this part of the PR.
The linked issue mentions some native calls that can be used as a workaround, and we could add them to Win32MppImpls in a separate PR. I don't think that's necessary, though, since the issue also mentions that proper PTY support was added in 2019. It looks like Mordant can detect interactivity correctly on modern versions of mintty, so I would say we could omit all the workarounds and let folks on very old versions of mintty live without color.
There was a problem hiding this comment.
Actually, my initial code didn't include this isMinttyTerminal() check. But I had to add it as otherwise the if (!interactive) return NONE in ansiLevel() is taken and the when (getTermProgram()) { isn't even reached...
So...
It looks like Mordant can detect interactivity correctly on modern versions of mintty
no, that did't work for me (using mintty 3.6.4).
This will cause ANSI codes to be output even if stdout is redirected to a file, which we want to avoid.
True, but that's also already the case for ij == true, or?
There was a problem hiding this comment.
There was a problem hiding this comment.
when (getTermProgram()) {isn't even reached...
Yes, that's the point; if stdout is redirected to a file, we don't want to output ansi codes.
that's also already the case for ij == true
The intellij run console is a special case since it's not a normal terminal: you can't redirect it's output to a file, so we know it's always interactive.
Also see https://github.com/mintty/mintty/wiki/Tips#inputoutput-interaction-with-alien-programs.
That seems to support the idea that PTY support should work on newer versions of mintty. Did you try setting and of the config/evvnars they mention?
There was a problem hiding this comment.
Yes, that's the point; if stdout is redirected to a file, we don't want to output ansi codes.
Right. But my point is that when (getTermProgram()) { isn't reached even if output is not redirected to a file.
The intellij run console is a special case
Ok, understood.
That seems to support the idea that PTY support should work on newer versions of mintty.
You have to read closely: That's only the case when running mintty on Cygwin / MSYS2, i.e. if it uses the cygwin1.dll emulation layer. It's not the case when running mintty as an MinGW program, like it is done in the Git Bash that comes with Git for Windows. That's why Git for Windows itself has this detection mechanism; we'd need something similar in Mordant.
Did you try setting and of the config/evvnars they mention?
No. I'd like Mordant to work out of the box with a fresh installation of Git for Windows / mintty without any need to explicitly tell it that it's a color console; such an approach would not scale across users.
There was a problem hiding this comment.
when (getTermProgram()) {isn't reached even if output is not redirected to a file.
That's correct. Whenever we're in a situation where it's not possible to detect whether output is interactive or not (which is the case for Git for Windows), we should assume it is not, in order to avoid printing ANSI codes to a file.
Git for Windows itself has this detection mechanism
Yeah, that was the workaround I mentioned that we could add to Win32MppImpls in a separate PR.
There was a problem hiding this comment.
So how should we move forward? IMO the preferred order would be to first have the means in Win32MppImpls to be able to implement that pipe-based detection, and then I'd rewrite this PR of mine to implement isMinttyTerminal() using those functions instead of just looking at the environment variable.
Do you agree?
There was a problem hiding this comment.
We might as well merge the envvar part now, since it's still valid for systems where the interactivity is detected correctly.
There was a problem hiding this comment.
Ok. I've rewritten my commit to not include the changes to the interactive logic.
There was a problem hiding this comment.
Yeah, that was the workaround I mentioned that we could add to
Win32MppImplsin a separate PR.
@ajalt are you going to do that?
| @@ -161,6 +170,14 @@ internal object TerminalDetection { | |||
| return ver != null && ver >= 3 | |||
| } | |||
|
|
|||
| private fun minttyVersionSupportsHyperlinks(): Boolean { | |||
095c0e1 to
3f0c1af
Compare
See http://mintty.github.io/. Note that when running mintty as a MingGW application (as done by Git Bash on Windows), the check for an interactive terminal currently fails as mintty is redirecting output, see [1] and [2]. This needs to be compensated with by a pipe-based check as done by Git for Windows itself [3]. [1]: mintty/mintty#482 [2]: https://github.com/mintty/mintty/wiki/Tips#inputoutput-interaction-with-alien-programs [3]: https://github.com/git/git/blob/d4a83d07b8cc66d4afac2f33a8af729f2ba93bba/compat/winansi.c#L579-L585
3f0c1af to
b30f22b
Compare
See http://mintty.github.io/. Note that when running mintty as a MingGW
application (as done by Git Bash on Windows), the check for an
interactive terminal currently fails as mintty is redirecting output,
see 1 and 2. This needs to be compensated with by a pipe-based
check as done by Git for Windows itself 3.