lf: Pressing w in lf crashes the terminal on macOS

This issue has been bothering me a lot lately. Unfortunately, there are no good news so far. But I felt like I should give a technical update on the issue.

There has been changes in tcell to use stdin/stdout instead of /dev/tty recently. I was hoping that could solve our issue but apparently not. With this recent change, shell commands are now broken on linux as well. The issue this time seems to be that input is consumed by tcell even after Screen.Fini is called. I have mentioned a small change in https://github.com/gdamore/tcell/issues/394 to quit tcell input loop when the screen is closed. However, the first character is still lost with this change which would be annoying to most users after a while. We might also need further changes to make this work as proper ui pause/resume. https://github.com/gdamore/tcell/issues/394 is now closed and at this point it’s not clear there will be any further changes to support such ui pause/resume behavior. There is also https://github.com/gdamore/tcell/issues/194 which is an older and more popular issue and it is also likely related to our issue but there hasn’t been any discussion there regarding the recent change yet.

So far I have failed to fix the lost first character issue with the most recent tcell. I have been trying non-blocking IO for stdin in tcell, which is difficult but not impossible in Go. The idea is to first syscall.Dup the stdin, then set syscall.SetNonblock the new file descriptor, and then create a new file handler with the descriptor using os.NewFile. Then it is possible to use os.File.SetDeadline in the new file to do non-blocking IO for stdin. I felt like this should work but it didn’t. If someone can make it work somehow, please let me know. Alternatively, if you can fix the issue on MacOS with the old /dev/tty tcell version, also let me know and we can maintain a separate patched fork with the old /dev/tty interface.

As a backup plan, I have also been thinking of adding another UI backend to lf as an option. We could have tried adding termbox back since we haven’t diverged much so far but discussion here suggests that this issue also existed with termbox (e.g. r16 and before). I don’t know any other pure Go terminal library besides tcell and termbox. Another strong alternative would be to use ncurses with cgo. As far as I know, terminal pause/resume behavior is explicitly supported in ncurses as mentioned here.

I have been trying shell commands with small toy ncurses programs. The following should work with go run foo.go if you have a c compiler and ncurses development files installed:

package main

// #include <ncurses.h>
// #cgo LDFLAGS: -lncurses
// static void nprintw(char* s) { printw(s); }
import "C"

import (
	"fmt"
	"os"
	"os/exec"
)

func shellout() {
	C.endwin()

	cmd := exec.Command("sh", "-c", "$SHELL", "--")
	cmd.Stdin = os.Stdin
	cmd.Stdout = os.Stdout
	cmd.Stderr = os.Stderr
	fmt.Println("spawning a shell... (try a few commands and then 'exit')")
	if err := cmd.Run(); err != nil {
		fmt.Println(err)
	} else {
		fmt.Println("shell is properly finished.")
	}

	C.initscr()
}

func draw() {
	C.move(1, 1)
	C.nprintw(C.CString("press 'w' to spawn a shell and 'q' to quit"))
}

func main() {
	C.initscr()
	draw()

loop:
	for {
		ch := C.getch()
		switch ch {
		case 'q':
			break loop
		case 'w':
			shellout()
		}
		draw()
	}

	C.endwin()
}

If you don’t have ncurses development files installed, the following python script should do something similar:

#!/usr/bin/env python

import curses
import subprocess

stdscr = curses.initscr()
stdscr.addstr(1, 1, "press 'w' to spawn a shell and 'q' to quit")

while True:
    key = stdscr.getkey()
    if key == 'q':
        break
    elif key == 'w':
        curses.endwin()
        subprocess.run(["sh", "-c", "$SHELL", "--"])
        stdscr = curses.initscr()
    stdscr.addstr(1, 1, "press 'w' to spawn a shell and 'q' to quit")

curses.endwin()

Some of the advantages of a possible ncurses backend are as follows:

1. Curses is part of POSIX standard as found here and it should come installed in most systems. I also expect it to be much more stable than any other terminal ui library out there. Since it is a commonly used library, people would be hesitant to make any changes in the system that may break curses programs.
2. Curses API might be more suitable for lf. For example with termbox/tcell API, we have to interpret color/attribute escape codes manually ourselves. Curses might allow additional terminal specific escape codes to work without any effort on our part. For example sixel graphics might simply work, which could be a nice alternative for true image previews.

And some disadvantages are as follows:

1. Curses requires cgo though this might already be inevitable. One of the things Go has done since the beginning was to avoid libc and have system calls implemented in the language. But this has been fading away recently. As far as I understand, on MacOS Go binaries are already expected to implement system calls through a library like libSystem. I have seen news that OpenBSD will also only allow system calls through libc for security reasons from Go1.16 onwards. Other BSDs might follow OpenBSD in the future. Most people expect Linux to stay the way it is. I’m not sure about Windows and others.
2. Curses might break static binaries. Currently we compile static binaries by disabling cgo. I think there are other methods to do this with build tags for pure Go. I’m not sure if it’s possible to statically link ncurses to Go binaries.
3. Curses might break our prebuilt binaries. One of the best things about Go is to be able to cross-compile for all supported systems from the same machine which we added to our continuous integration system to automatically build prebuilt binaries for each release. Adding cgo might prevent cross-compilation.
4. Curses may not work on Windows. I remember a windows port for curses but I don’t know the situtation nowadays.
5. Curses backend requires a significant effort to implement. To give an idea, our ui.go is over 1200 lines and we have some further tcell related code in other places such as colors.go. Maybe about 1/3 of this code might be considered ui independent which can be refactored out. Maybe 1/3 might require slight changes to make them ui independent. And the rest should be implemented specifically for curses. Unfortunately I can only work on lf occasionally these days so it may not be possible for me to work on this anytime soon but maybe contributors can help with this instead if we actually decide in favor of this addition.
6. Maintaining multiple UIs might be difficult. They will probably not have feature parity. They will also likely double our ui related issues.
7. Curses might have some quirks regarding input handling and unicode support.

Note that I’m actually considering curses as an optional build feature using build tags (e.g. go build +curses) so disadvantages 1, 2, 3, and 4 may not actually be much of an issue. Disadvantages 5, 6, and 7 are more serious issues to be considered. Feel free to leave feedback about curses backend for discussion.

Nobody has reported anything so I have made a new release r21 with the fix.

There is an EventError fired after cmd.Run which shows up in our log files as:

EventError: 'read /dev/stdin: resource temporarily unavailable'

But it seems harmless. We could disable event error logging to get rid of it but it could be useful elsewhere so I haven’t changed anything.

Closing this issue.

@MunifTanjim I have seen it before but I think it wasn’t clear whether it is against TOS or not. In any case, I think the discussion on tcell side is going in the direction to fix the issue in a different way.

@jeffs Sorry for misleading you with the log file location. I guess that’s the path Mac uses for temp files.

@jeffs @thezeroalpha It is unfortunate that I don’t have a Mac to investigate this issue. I have marked the issue with help wanted tag to hopefully attract a Mac developer to work on this. In the meantime, you may try to get used to lfcd and quit workflow rather than spawning a shell inside lf.

Your issues sound similar in some ways but also different in some others. It might be the case that you both have different MacOS versions and there might have been tty related changes recently in MacOS. We don’t directly manipulate tty in lf so it is highly possible that this is a termbox/tcell related problem. If you have Go installed, I may try to come up with a small toy program for you to run and confirm that this is the case, and then we can report the issue back to tcell repo, where devs are more likely to have a better understanding of the problem.