From 3dee8778d073199e0fe1e186e54f7eabc2fdef43 Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Thu, 23 May 2024 20:08:20 +0900 Subject: [PATCH] execute: Open separate handles to /dev/tty (in, out, err) # This will no longer cause 'Vim: Warning: Output is not to a terminal' fzf --bind 'enter:execute:vim {}' > /tmp/foo --- src/proxy.go | 8 ++++++-- src/reader.go | 2 +- src/terminal.go | 38 ++++++++++++++++++++++++++++++++++---- src/tui/light.go | 8 ++------ src/tui/light_unix.go | 38 ++++---------------------------------- src/tui/tcell_test.go | 3 ++- src/tui/ttyname_unix.go | 25 ++++++++++++++++++------- src/tui/ttyname_windows.go | 13 ++++++++++--- src/util/util.go | 11 +++-------- 9 files changed, 80 insertions(+), 66 deletions(-) diff --git a/src/proxy.go b/src/proxy.go index adeea21..5ee7317 100644 --- a/src/proxy.go +++ b/src/proxy.go @@ -60,7 +60,7 @@ func runProxy(commandPrefix string, cmdBuilder func(temp string) *exec.Cmd, opts var command string commandPrefix += ` --proxy-script "$0"` - if opts.Input == nil && util.IsTty() { + if opts.Input == nil && util.IsTty(os.Stdin) { command = fmt.Sprintf(`%s > %q`, commandPrefix, output) } else { input, err := fifo("proxy-input") @@ -131,7 +131,11 @@ func runProxy(commandPrefix string, cmdBuilder func(temp string) *exec.Cmd, opts env = elems[1:] } executor := util.NewExecutor(opts.WithShell) - executor.Become(tui.TtyIn(), env, command) + ttyin, err := tui.TtyIn() + if err != nil { + return ExitError, err + } + executor.Become(ttyin, env, command) } return code, err } diff --git a/src/reader.go b/src/reader.go index 99a609e..83912df 100644 --- a/src/reader.go +++ b/src/reader.go @@ -113,7 +113,7 @@ func (r *Reader) ReadSource(inputChan chan string, root string, opts walkerOpts, var success bool if inputChan != nil { success = r.readChannel(inputChan) - } else if util.IsTty() { + } else if util.IsTty(os.Stdin) { cmd := os.Getenv("FZF_DEFAULT_COMMAND") if len(cmd) == 0 { success = r.readFiles(root, opts, ignores) diff --git a/src/terminal.go b/src/terminal.go index 58073dd..24c9cdd 100644 --- a/src/terminal.go +++ b/src/terminal.go @@ -51,6 +51,7 @@ var whiteSuffix *regexp.Regexp var offsetComponentRegex *regexp.Regexp var offsetTrimCharsRegex *regexp.Regexp var passThroughRegex *regexp.Regexp +var ttyin *os.File const clearCode string = "\x1b[2J" @@ -691,11 +692,19 @@ func NewTerminal(opts *Options, eventBox *util.EventBox, executor *util.Executor var renderer tui.Renderer fullscreen := !opts.Height.auto && (opts.Height.size == 0 || opts.Height.percent && opts.Height.size == 100) var err error + // Reuse ttyin if available to avoid having multiple file descriptors open + // when you run fzf multiple times in your Go program. Closing it is known to + // cause problems with 'become' action and invalid terminal state after exit. + if ttyin == nil { + if ttyin, err = tui.TtyIn(); err != nil { + return nil, err + } + } if fullscreen { if tui.HasFullscreenRenderer() { renderer = tui.NewFullscreenRenderer(opts.Theme, opts.Black, opts.Mouse) } else { - renderer, err = tui.NewLightRenderer(opts.Theme, opts.Black, opts.Mouse, opts.Tabstop, opts.ClearOnExit, + renderer, err = tui.NewLightRenderer(ttyin, opts.Theme, opts.Black, opts.Mouse, opts.Tabstop, opts.ClearOnExit, true, func(h int) int { return h }) } } else { @@ -711,7 +720,7 @@ func NewTerminal(opts *Options, eventBox *util.EventBox, executor *util.Executor effectiveMinHeight += borderLines(opts.BorderShape) return util.Min(termHeight, util.Max(evaluateHeight(opts, termHeight), effectiveMinHeight)) } - renderer, err = tui.NewLightRenderer(opts.Theme, opts.Black, opts.Mouse, opts.Tabstop, opts.ClearOnExit, false, maxHeightFunc) + renderer, err = tui.NewLightRenderer(ttyin, opts.Theme, opts.Black, opts.Mouse, opts.Tabstop, opts.ClearOnExit, false, maxHeightFunc) } if err != nil { return nil, err @@ -818,7 +827,7 @@ func NewTerminal(opts *Options, eventBox *util.EventBox, executor *util.Executor serverOutputChan: make(chan string), eventChan: make(chan tui.Event, 6), // (load + result + zero|one) | (focus) | (resize) | (GetChar) tui: renderer, - ttyin: tui.TtyIn(), + ttyin: ttyin, initFunc: func() error { return renderer.Init() }, executing: util.NewAtomicBool(false), lastAction: actStart, @@ -2874,9 +2883,30 @@ func (t *Terminal) executeCommand(template string, forcePlus bool, background bo cmd.Env = t.environ() t.executing.Set(true) if !background { - cmd.Stdin = t.ttyin + // Open a separate handle for tty input + if in, _ := tui.TtyIn(); in != nil { + cmd.Stdin = in + if in != os.Stdin { + defer in.Close() + } + } + cmd.Stdout = os.Stdout + if !util.IsTty(os.Stdout) { + if out, _ := tui.TtyOut(); out != nil { + cmd.Stdout = out + defer out.Close() + } + } + cmd.Stderr = os.Stderr + if !util.IsTty(os.Stderr) { + if out, _ := tui.TtyOut(); out != nil { + cmd.Stderr = out + defer out.Close() + } + } + t.tui.Pause(true) cmd.Run() t.tui.Resume(true, false) diff --git a/src/tui/light.go b/src/tui/light.go index 1181d16..f202899 100644 --- a/src/tui/light.go +++ b/src/tui/light.go @@ -127,11 +127,7 @@ type LightWindow struct { bg Color } -func NewLightRenderer(theme *ColorTheme, forceBlack bool, mouse bool, tabstop int, clearOnExit bool, fullscreen bool, maxHeightFunc func(int) int) (Renderer, error) { - in, err := openTtyIn() - if err != nil { - return nil, err - } +func NewLightRenderer(ttyin *os.File, theme *ColorTheme, forceBlack bool, mouse bool, tabstop int, clearOnExit bool, fullscreen bool, maxHeightFunc func(int) int) (Renderer, error) { out, err := openTtyOut() if err != nil { out = os.Stderr @@ -142,7 +138,7 @@ func NewLightRenderer(theme *ColorTheme, forceBlack bool, mouse bool, tabstop in forceBlack: forceBlack, mouse: mouse, clearOnExit: clearOnExit, - ttyin: in, + ttyin: ttyin, ttyout: out, yoffset: 0, tabstop: tabstop, diff --git a/src/tui/light_unix.go b/src/tui/light_unix.go index 8d5a279..06099d2 100644 --- a/src/tui/light_unix.go +++ b/src/tui/light_unix.go @@ -7,7 +7,6 @@ import ( "os" "os/exec" "strings" - "sync" "syscall" "github.com/junegunn/fzf/src/util" @@ -15,13 +14,6 @@ import ( "golang.org/x/term" ) -var ( - tty string - ttyin *os.File - ttyout *os.File - mutex sync.Mutex -) - func IsLightRendererSupported() bool { return true } @@ -53,15 +45,13 @@ func (r *LightRenderer) initPlatform() error { } func (r *LightRenderer) closePlatform() { - // NOOP + r.ttyout.Close() } func openTty(mode int) (*os.File, error) { in, err := os.OpenFile(consoleDevice, mode, 0) if err != nil { - if len(tty) == 0 { - tty = ttyname() - } + tty := ttyname() if len(tty) > 0 { if in, err := os.OpenFile(tty, mode, 0); err == nil { return in, nil @@ -73,31 +63,11 @@ func openTty(mode int) (*os.File, error) { } func openTtyIn() (*os.File, error) { - mutex.Lock() - defer mutex.Unlock() - - if ttyin != nil { - return ttyin, nil - } - in, err := openTty(syscall.O_RDONLY) - if err == nil { - ttyin = in - } - return in, err + return openTty(syscall.O_RDONLY) } func openTtyOut() (*os.File, error) { - mutex.Lock() - defer mutex.Unlock() - - if ttyout != nil { - return ttyout, nil - } - out, err := openTty(syscall.O_WRONLY) - if err == nil { - ttyout = out - } - return out, err + return openTty(syscall.O_WRONLY) } func (r *LightRenderer) setupTerminal() { diff --git a/src/tui/tcell_test.go b/src/tui/tcell_test.go index 54b9c9b..217ad04 100644 --- a/src/tui/tcell_test.go +++ b/src/tui/tcell_test.go @@ -3,6 +3,7 @@ package tui import ( + "os" "testing" "github.com/gdamore/tcell/v2" @@ -20,7 +21,7 @@ func assert(t *testing.T, context string, got interface{}, want interface{}) boo // Test the handling of the tcell keyboard events. func TestGetCharEventKey(t *testing.T) { - if util.ToTty() { + if util.IsTty(os.Stdout) { // This test is skipped when output goes to terminal, because it causes // some glitches: // - output lines may not start at the beginning of a row which makes diff --git a/src/tui/ttyname_unix.go b/src/tui/ttyname_unix.go index 384115f..d0350a0 100644 --- a/src/tui/ttyname_unix.go +++ b/src/tui/ttyname_unix.go @@ -4,12 +4,19 @@ package tui import ( "os" + "sync/atomic" "syscall" ) var devPrefixes = [...]string{"/dev/pts/", "/dev/"} +var tty atomic.Value + func ttyname() string { + if cached := tty.Load(); cached != nil { + return cached.(string) + } + var stderr syscall.Stat_t if syscall.Fstat(2, &stderr) != nil { return "" @@ -27,17 +34,21 @@ func ttyname() string { continue } if stat, ok := info.Sys().(*syscall.Stat_t); ok && stat.Rdev == stderr.Rdev { - return prefix + file.Name() + value := prefix + file.Name() + tty.Store(value) + return value } } } return "" } -// TtyIn returns terminal device to be used as STDIN, falls back to os.Stdin -func TtyIn() *os.File { - if in, err := openTtyIn(); err == nil { - return in - } - return os.Stdin +// TtyIn returns terminal device to read user input +func TtyIn() (*os.File, error) { + return openTtyIn() +} + +// TtyIn returns terminal device to write to +func TtyOut() (*os.File, error) { + return openTtyOut() } diff --git a/src/tui/ttyname_windows.go b/src/tui/ttyname_windows.go index 39b84f7..0313c60 100644 --- a/src/tui/ttyname_windows.go +++ b/src/tui/ttyname_windows.go @@ -2,13 +2,20 @@ package tui -import "os" +import ( + "os" +) func ttyname() string { return "" } // TtyIn on Windows returns os.Stdin -func TtyIn() *os.File { - return os.Stdin +func TtyIn() (*os.File, error) { + return os.Stdin, nil +} + +// TtyIn on Windows returns nil +func TtyOut() (*os.File, error) { + return nil, nil } diff --git a/src/util/util.go b/src/util/util.go index 9b926b8..ec5a1ea 100644 --- a/src/util/util.go +++ b/src/util/util.go @@ -138,17 +138,12 @@ func DurWithin( return val } -// IsTty returns true if stdin is a terminal -func IsTty() bool { - fd := os.Stdin.Fd() +// IsTty returns true if the file is a terminal +func IsTty(file *os.File) bool { + fd := file.Fd() return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) } -// ToTty returns true if stdout is a terminal -func ToTty() bool { - return isatty.IsTerminal(os.Stdout.Fd()) -} - // Once returns a function that returns the specified boolean value only once func Once(nextResponse bool) func() bool { state := nextResponse