check cygwin pty

[mattn]

@koron @k-takata

[k-takata]

LGTM :+1:

[mattn]

jvgrep でしばらく実験して、うまく行きそうなら

• jvgrep から取り除いて
• go-isatty へ取り込み

と行きたいです。go-isatty や go-colorable は色んなプロジェクトで使われすぎていて(dockerとか)最近変更が怖いので、慎重にやらせて下さい。

[mattn]

linux でのビルドと動作(現状通り)を確認。

[mattn]

@kozo2 homebrew のリリースに何か影響あったりしますか？依存パッケージが1つ増えます。

[k-takata]

• jvgrep から取り除いて
• go-isatty へ取り込み

これについてですが、次のように go-colorable の修正も必要と考えていますが、合っていますか？

• isatty.IsTerminal() を修正し、IsCygwinPty() が true の場合でも true を返すようにする。
• 現状の go-colorable の Windows 版では単純に、isatty.IsTerminal() が ture であれば、Win32 Console API を呼ぶようになっているが、IsCygwinPty() が true の場合はエスケープシーケンスをそのまま流すようにする。

[mattn]

そうですね。2個ハードルがありますね。Cygwin/MSYS2 での動作と引き換えに依存が増える事を望んでいないユーザが果たしているかどうかが気になっています。僕はいいと思ってるんですがたまにそういう人もいます。

[k-takata]

手元では、本PRと、go-colorable に対する以下の修正で、mintty上でカラー表示できることが確認できました。

--- a/colorable_windows.go
+++ b/colorable_windows.go
@@ -12,6 +12,7 @@ import (
    "unsafe"

    "github.com/mattn/go-isatty"
+   "github.com/k-takata/go-iscygpty"
 )

 const (
@@ -80,7 +81,9 @@ func NewColorable(file *os.File) io.Writer {
        panic("nil passed instead of *os.File to NewColorable()")
    }

-   if isatty.IsTerminal(file.Fd()) {
+   if iscygpty.IsCygwinPty(file.Fd()) {
+       return file
+   } else if isatty.IsTerminal(file.Fd()) {
        var csbi consoleScreenBufferInfo
        handle := syscall.Handle(file.Fd())
        procGetConsoleScreenBufferInfo.Call(uintptr(handle), uintptr(unsafe.Pointer(&csbi)))

jvgrepだけ修正するのであれば、この修正はなくても動くはずですが、isttyも修正するのであれば必須と思われます。

[kozo2]

homebrew の formula で https://github.com/mattn/jvgrep/archive/iscygpty.zip を見させる方法がわかりませんでした。URL内に必ずバージョンを表す数字を求める仕様のようです。 renameしてどっかのpublicなfile serverに置ければ確かめることはできると思うのですができてません。 依存パッケージが1つ増えることはhomebrew的には問題無いかと思います。

[mattn]

依存パッケージが1つ増えることはhomebrew的には問題無いかと思います。

了解しました。そのままこの pr をマージすればいいという理解です。ありがとうございます。

[mattn]

merged https://github.com/mattn/jvgrep/commit/224a34dce953c49073768b49b7d6eef6f96aa090

[kozo2]

@mattn ちなみに homebrew の jvgrep formula は最近だと私より @ilovezfs さんが先に更新してくださってます

[ilovezfs]

It is now released :) https://github.com/Homebrew/homebrew-core/commit/cfd3e42ae4afad3b93645fb12c38385135b1c988 https://github.com/Homebrew/homebrew-core/commit/29a47143b8fee3847f51444ac9c1b4a70dfd82c3

:tada: