-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
automatic tty redirection should not statically target /dev/tty, but the current pty/pts #4242
Comments
Have you actually had the problem while using It's odd because fzf writes directly to Line 30 in ac32fbb
|
Yes emacsclient will always fail with simple /dev/tty redirection.
I guess the problem is, that it connects to the emacsdaemon, which no longer has access to a controlling terminal, but still can make use of the concrete pty device node you can obtain by asking $(tty). This is probably also the reason why the non-deamon version of emacs will work in this case. This is not specific to emacsclient though, it's just the most likely candidate for me to observe this problem. |
Can you explain how I can test fzf in emacs? I've never used emacs before. |
Unfortunately, this approach can't be used. Because fzf is a filter, stdin is usually not the tty device. $ ls | fzf
$ tty
/dev/ttys009
$ ls | tty
not a tty We could make fzf check if a variable named |
Ah right, sorry good point. Since fzf requires to have a controlling terminal, we could just keep using that to learn the pty, that we need to provide to the children fd, err := unix.Open("/dev/tty", unix.O_RDWR, 0)
consoleDevice, err := unix.Ttyname(fd) |
Can you show me how I can reproduce the problem and see if the patch helps? Like I said, no experience in Emacs. |
Sure, i really should have added this to the first message already, sorry:
emacs --daemon
CURRENT_TTY=$(tty)
ls | fzf --bind "space:execute:emacsclient -t -a \"\" -c {} < $CURRENT_TTY > $CURRENT_TTY" > selected
ls | fzf --bind "space:execute:emacsclient -t -a \"\" -c {} < /dev/tty > /dev/tty" > selected
*ERROR*: Could not open file: /dev/tty |
Does this code work? There is no Lines 15 to 21 in 6fa8295
But I couldn't get it to work. |
Ah sorry. Looks like I led you into a wrong direction. Getting the ttyname of /dev/tty will usually still return /dev/tty, instead of the actual pty. Instead when allocating a new PTY, everything seems to work as intended. Even with --tmux. diff --git a/src/tui/light_unix.go b/src/tui/light_unix.go
index 76aac2eb..447c0a13 100644
--- a/src/tui/light_unix.go
+++ b/src/tui/light_unix.go
@@ -2,6 +2,15 @@
package tui
+/*
+#define _XOPEN_SOURCE 600
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <errno.h>
+*/
+import "C"
+
import (
"errors"
"os"
@@ -42,8 +51,30 @@ func (r *LightRenderer) closePlatform() {
r.ttyout.Close()
}
+func open_pty() (processTTY string, err error) {
+
+ m, err := C.posix_openpt(C.O_RDWR | C.O_NOCTTY)
+ if m < 0 {
+ return "", err
+ }
+ defer C.close(m)
+ if res, err := C.grantpt(m); res < 0 {
+ return "", err
+ }
+ if res, err := C.unlockpt(m); res < 0 {
+ return "", err
+ }
+ processTTY = C.GoString(C.ptsname(m))
+ return processTTY, nil
+}
+
func openTty(mode int) (*os.File, error) {
- in, err := os.OpenFile(consoleDevice, mode, 0)
+ ptyname, err := open_pty()
+ if err != nil {
+ ptyname = consoleDevice
+ }
+
+ in, err := os.OpenFile(ptyname, mode, 0)
if err != nil {
tty := ttyname()
if len(tty) > 0 {
The code above is mostly a copy of pty_cgo.go |
Thanks, is there any way we can do it without cgo? I remember we had moved away from using cgo for various reasons (e.g. trouble with cross compiling), and I don't want to bring it back. |
Checklist
man fzf
)Output of
fzf --version
0.59.0 (bbe1721)
OS
Shell
Problem / Steps to reproduce
Since 0.53.0 fzf will automatically redirect execute actions to /dev/tty,
so that the usual hack
could be simplified, without worrying about any redirection
but the static /dev/tty is not universally understood/not always a working solution.
A more robust approach would be to query the current pty (e.g. with /usr/bin/tty), which would return something like /dev/pts/4
The more general solution, that also works with programs like emacsclient, should probably be equivalent to something like this:
The text was updated successfully, but these errors were encountered: