haskell / win32

Haskell support for the Win32 API
http://hackage.haskell.org/package/Win32
Other
97 stars 63 forks source link

`isMinTTY` no longer detects recent `mintty`s #186

Closed RyanGlScott closed 3 years ago

RyanGlScott commented 3 years ago

isMinTTY incorrectly returns false on recent versions of mintty.

Current Behavior/Steps to Reproduce

To reproduce, run the following in GHCi:

> import System.Win32.MinTTY
> isMinTTY
False
> :! mintty --version
mintty 3.5.1 (x86_64-pc-msys)
© 2013/2021 Andy Koppe / Thomas Wolff
License GPLv3+: GNU GPL version 3 or later
There is no warranty, to the extent permitted by law.

Your Environment

RyanGlScott commented 3 years ago

With some printf debugging:

diff --git a/System/Win32/MinTTY.hsc b/System/Win32/MinTTY.hsc
index 97218b0..ffdb749 100644
--- a/System/Win32/MinTTY.hsc
+++ b/System/Win32/MinTTY.hsc
@@ -79,6 +79,7 @@ isMinTTYHandle h = do
 isMinTTYVista :: HANDLE -> IO Bool
 isMinTTYVista h = do
     fn <- getFileNameByHandle h
+    putStrLn fn
     return $ cygwinMSYSCheck fn
   `catch` \(_ :: IOError) ->
     return False

I get:

> isMinTTY
\msys-dd50a72ab4668b33-pty0-to-master-nat
False

Here is how cygwinMSYSCheck is currently implemented:

https://github.com/haskell/win32/blob/cd69b1944f1afa4f85bca314e91c56f00cf3f921/System/Win32/MinTTY.hsc#L93-L98

This assumes that -master will be at the very end, but in this case, we have -master-nat. I wonder what the right thing to do is? Here is what the Rust atty library does:

    // This checks whether 'pty' exists in the file name, which indicates that
    // a pseudo-terminal is attached. To mitigate against false positives
    // (e.g., an actual file name that contains 'pty'), we also require that
    // either the strings 'msys-' or 'cygwin-' are in the file name as well.)
    let is_msys = name.contains("msys-") || name.contains("cygwin-");
    let is_pty = name.contains("-pty");
    is_msys && is_pty

git-for-windows does something similar.

Perhaps we should remove the -master check? I blindly copied that part from the MinTTY detection code from GHC at the time, but perhaps that part was never really correct to begin with.

Mistuke commented 3 years ago

Hi, thanks for looking into this,

Dropping the master check seems sensible to me. The one thing I'm wondering about is whether we still do the right thing on newer mintty. Specifically newer mintty have support for conpty[1] which is an actual pty without the quirks of mintty.

https://github.com/mintty/mintty/issues/804

So my question is if the behavior we're guarding against with these checks are still valid.

But I guess that's a different question..

[1] https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/

RyanGlScott commented 3 years ago

Interesting. What is the easiest way to enable ConPTY support in mintty so that I could test it?

Mistuke commented 3 years ago

You have to have an msys runtime based on cygwin 3.1.0 and up, so uname -r should be 3.1.0+, you can upgrade with pacman -Syuu

After upgrading the runtime you can upgrade your packages (repeat the same command) which should get you an updated mintty.

In particular with conpty support you should be able to use any terminal, like Microsoft Terminal with msys2 and it should all work the same as MinTTY.

RyanGlScott commented 3 years ago

As far as I can tell, I already have a sufficiently recent version of Cygwin:

$ uname -r
3.2.0-340.x86_64

If I launch cmd.exe from within MSYS2's mintty and repeat the same exercise, I get:

> isMinTTY
\msys-dd50a72ab4668b33-pty0-to-master-nat
False
Mistuke commented 3 years ago

As far as I can tell, I already have a sufficiently recent version of Cygwin:

$ uname -r
3.2.0-340.x86_64

If I launch cmd.exe from within MSYS2's mintty and repeat the same exercise, I get:

> isMinTTY
\msys-dd50a72ab4668b33-pty0-to-master-nat
False

Ah that's not what I meant. So currently these mintty detections are mostly being used to know when we need to behave differently as MinTTY was not a true tty. But when it's backed by conpty it should be.

Basically I was just wondering (out loud, not specifically to you) whether GHC(i) still needs to enable "mintty" mode

RyanGlScott commented 3 years ago

Even if MinTTY does support ConPTY, GHCi still behaves quite differently under MinTTY than it does other Windows consoles. For instance, GHCi's tab completion and Ctrl-C handling are still completely broken under MinTTY.