Skip to content

Commit

Permalink
Use MSYS=enable_pcon instead of winpty on mintty 3.4.5 or later
Browse files Browse the repository at this point in the history
  • Loading branch information
junegunn committed May 23, 2024
1 parent bfe2bf4 commit d4216b0
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 5 deletions.
7 changes: 2 additions & 5 deletions src/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package fzf

import (
"os"
"os/exec"
"sync"
"time"

Expand All @@ -25,10 +24,8 @@ func Run(opts *Options) (int, error) {
return runTmux(os.Args, opts)
}

if os.Getenv("TERM_PROGRAM") == "mintty" && !opts.NoWinpty {
if _, err := exec.LookPath("winpty"); err == nil {
return runWinpty(os.Args, opts)
}
if needWinpty(opts) {
return runWinpty(os.Args, opts)
}

if err := postProcessOptions(opts); err != nil {
Expand Down
32 changes: 32 additions & 0 deletions src/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package util
import (
"math"
"os"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -189,3 +190,34 @@ func ToKebabCase(s string) string {
}
return strings.ToLower(name)
}

// CompareVersions compares two version strings
func CompareVersions(v1, v2 string) int {
parts1 := strings.Split(v1, ".")
parts2 := strings.Split(v2, ".")

atoi := func(s string) int {
n, e := strconv.Atoi(s)
if e != nil {
return 0
}
return n
}

for i := 0; i < Max(len(parts1), len(parts2)); i++ {
var p1, p2 int
if i < len(parts1) {
p1 = atoi(parts1[i])
}
if i < len(parts2) {
p2 = atoi(parts2[i])
}

if p1 > p2 {
return 1
} else if p1 < p2 {
return -1
}
}
return 0
}
31 changes: 31 additions & 0 deletions src/util/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,3 +203,34 @@ func TestStringWidth(t *testing.T) {
t.Errorf("Expected: %d, Actual: %d", 1, w)
}
}

func TestCompareVersions(t *testing.T) {
assert := func(a, b string, expected int) {
if result := CompareVersions(a, b); result != expected {
t.Errorf("Expected: %d, Actual: %d", expected, result)
}
}

assert("2", "1", 1)
assert("2", "2", 0)
assert("2", "10", -1)

assert("2.1", "2.2", -1)
assert("2.1", "2.1.1", -1)

assert("1.2.3", "1.2.2", 1)
assert("1.2.3", "1.2.3", 0)
assert("1.2.3", "1.2.3.0", 0)
assert("1.2.3", "1.2.4", -1)

// Different number of parts
assert("1.0.0", "1", 0)
assert("1.0.0", "1.0", 0)
assert("1.0.0", "1.0.0", 0)
assert("1.0", "1.0.0", 0)
assert("1", "1.0.0", 0)
assert("1.0.0", "1.0.0.1", -1)
assert("1.0.0.1.0", "1.0.0.1", 0)

assert("", "3.4.5", -1)
}
4 changes: 4 additions & 0 deletions src/winpty.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ package fzf

import "errors"

func needWinpty(_ *Options) bool {
return false
}

func runWinpty(_ []string, _ *Options) (int, error) {
return ExitError, errors.New("Not supported")
}
49 changes: 49 additions & 0 deletions src/winpty_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,46 @@ import (
"fmt"
"os"
"os/exec"

"github.com/junegunn/fzf/src/util"
)

func isMintty345() bool {
return util.CompareVersions(os.Getenv("TERM_PROGRAM_VERSION"), "3.4.5") >= 0
}

func needWinpty(opts *Options) bool {
if os.Getenv("TERM_PROGRAM") != "mintty" {

This comment has been minimized.

Copy link
@Konfekt

Konfekt May 23, 2024

Contributor

There might obviously be others, but Windows Terminal is the new Windows standard and works fine, whereas mintty is the Git Bash standard, so I think this suffices

return false
}
if isMintty345() {
/*
See: https://github.com/junegunn/fzf/issues/3809
"MSYS=enable_pcon" allows fzf to run properly on mintty 3.4.5 or later,
however `--height` option still doesn't work, so let's just disable it.
We're not going to worry too much about restoring the original value.

This comment has been minimized.

Copy link
@Konfekt

Konfekt May 23, 2024

Contributor

Is this maybe not much of a concern thanks to the child process anyway?

This comment has been minimized.

Copy link
@junegunn

junegunn May 23, 2024

Author Owner

For environment variable, yes.

Restoring options is only needed when you use fzf as a library in a Go program and starts it multiple times, but I think unsetting --height and not restoring is totally fine, because without it, fzf will not work and you don't want that anyway.

*/
if os.Getenv("MSYS") == "enable_pcon" {

This comment has been minimized.

Copy link
@Konfekt

Konfekt May 23, 2024

Contributor

Likely MSYS accepts a list of options, so rather match \benable_pcon\b?

This comment has been minimized.

Copy link
@junegunn

junegunn May 23, 2024

Author Owner

I see. Let's keep it simple and use strings.Contains.

opts.Height = heightSpec{}
return false
}

// Setting the environment variable here unfortunately doesn't help,
// so we need to start a child process with "MSYS=enable_pcon"
// os.Setenv("MSYS", "enable_pcon")
return true
}
if _, err := exec.LookPath("winpty"); err != nil {
return false
}
if opts.NoWinpty {

This comment has been minimized.

Copy link
@Konfekt

Konfekt May 23, 2024

Contributor

Maybe switch place with the last if statement?

This comment has been minimized.

Copy link
@junegunn

junegunn May 23, 2024

Author Owner

Yep.

return false
}
return true
}

func runWinpty(args []string, opts *Options) (int, error) {
sh, err := sh()
if err != nil {
Expand All @@ -20,6 +58,17 @@ func runWinpty(args []string, opts *Options) (int, error) {
}
argStr += ` --no-winpty --no-height`

if isMintty345() {
return runProxy(argStr, func(temp string) *exec.Cmd {
cmd := exec.Command(sh, temp)
cmd.Env = append(os.Environ(), "MSYS=enable_pcon")
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
return cmd
}, opts, false)
}

return runProxy(argStr, func(temp string) *exec.Cmd {
cmd := exec.Command(sh, "-c", fmt.Sprintf(`winpty < /dev/tty > /dev/tty -- sh %q`, temp))
cmd.Stdout = os.Stdout
Expand Down

0 comments on commit d4216b0

Please sign in to comment.