lfcd.sh runs in subshell - does not work well with tmux and {pane_current_path}
Closed this issue ยท 11 comments
The lfcd function provided by this repo is currently.
lfcd () {
cd "$(command lf -print-last-dir "$@")"
}
However, this runs lf in a subshell. This is problematic because when using tmux and opening a new pane (using the pane_current_path), tmux does not navigate to the current folder; instead, the new pane navigates to the current directory when lf was initially run.
We have a solution that appears to work for now (shout out to @3ximus for helping figure this out).
lfcd () {
TMPFILE=$(mktemp /tmp/lfcd-XXXXXX)
\lf -print-last-dir $@ > $TMPFILE
cd $(cat $TMPFILE)
rm $TMPFILE
}
If acceptable, this or something with similar functionality should be the new lfcd suggestion.
We actually tried to remove the use of tempfiles in lfcd
because another user was complaining about lack of POSIX compatibility in #1393, but at the time we weren't aware of any issues like this.
Can you please provide a minimal config and steps to reproduce?
Interesting! Not sure what the best fix is, then. Perhaps there is another way without the temp files. But for now, I managed to reproduce the issue in a fresh Ubuntu VM; here are the steps:
Setup
Install tmux and the latest lf
sudo apt install tmux
sudo apt install golang-go -y
env CGO_ENABLED=0 go install -ldflags="-s -w" github.com/gokcehan/lf@latest
Create minimal config files. Note for tmux, I rebound the split to use the pane_current_path.
echo 'export PATH="$(go env GOPATH)/bin:$PATH"' >> .bashrc
source .bashrc
touch .tmux.conf
echo 'bind \" split-window -v -c "#{pane_current_path}"' >> .tmux.conf
tmux
tmux source-file .tmux.conf
Reproduce issue
- open tmux
- run lf
- navigate to any folder
- use the hotkey ctrl+b -> ctrl+" to open a split pane
The new pane will open with the path of your current lf directory; this is the behavior we want :)
Now do it again with lfcd
- open tmux
- this time, run
cd "$(command \lf -print-last-dir "$@")"
- navigate to any folder
- use the hotkey ctrl+b -> ctrl+"
Now, the new pane will open with the path you were in when you entered the lfcd command. :(
Thanks. I don't use tmux
very much myself so I'm not that familiar with using it.
Anyway I was able to reproduce the issue - for testing purposes I ended up displaying pane_current_path
in the status bar using the following config:
set -g status-right "#{pane_current_command} #{pane_current_path}"
set -g status-interval 1
I think the issue comes down to how tmux
evaluates pane_current_path
, and I'm guessing it is depends on lf
was invoked. It works fine if lf
is the first command running (e.g. lf
or lf | :
), but not in other cases (e.g. $(lf)
or : | lf
). The problem is that for cd
to work, it also has to be the first command (i.e. cd "$(...)"
).
I can't think of a good solution without making some kind of compromise:
- Accepting that
tmux
won't evaluatepane_current_path
as expected - Using
mktemp
and breaking POSIX compatibility - Using a hardcoded path which may or may not exist on the user's system
- Increasing the complexity of
lfcd
by doing something different based on whether it is running intmux
or not - Adding the
mktemp
implementation tolfcd.sh
as a comment, but then users will have to manually uncomment it if they want to use that version instead
One other thing you can try is to call split-window
from inside lf
like below. That should work but it just means you have to memorize a couple more keybindings.
map _ %tmux split-window -v
map | %tmux split-window -h
Also I'm not sure if it helps, but lf
does contain an on-cd
hook which is called whenever you change directory inside it. Maybe there's a way to use it to tell tmux
what the new directory is, but I'm not sure.
Thanks for the comments! It's very interesting; I never knew this limitation of bash.
Mapping the tmux
split window within lf
does work, but it is less ideal for my personal use. For me, I'm pretty happy with the solution I posted; I guess the bigger question is how lfcd.sh
should be shipped.
I'm not well-versed in how important POSIX compatibility is for this project. All I can say from a user point of view it would be nice to have access to this alternative. As is, this issue happens "silently" - until I saw someone else use it, I thought this was a limitation of lf
not lfcd.sh
. It's possible that the number of users that use tmux
with pane_current_path
and lfcd.sh
is relatively niche so perhaps the last option is best with a comment like # Use this for better compatibility with tmux
It's certainly less elegant than the current solution, which is too bad.
Yeah the main problem is that it's possible for a pane to have multiple processes running (e.g. via a shell pipeline), and since each process has its own CWD, tmux
has to 'select' one for the purposes of evaluating pane_current_path
.
Regarding POSIX compatibility, I am not extremely uptight about it - TBH I advocated for the cd $(lf ...)
approach just because it was more elegant. But I am fine with you adding a mktemp
version in lfcd.sh
if you would like to do that.
That sounds good. I'll create a pull request as soon as I get a chance.
How would this work? This way, if mktemp does not exist or fails, it reverts to the current method.
lfcd () {
# check if mktemp command exists, and if so, generates the file name
if hash mktemp ; then tmp=$(mktemp /tmp/lfcdXXXXXXXX) ; fi
# check if tmp file was created otherwise, revert POSIX compliant option
if [ -f "$tmp" ] ; then
\lf -print-last-dir "$@" > "$tmp" && cd $(cat "$tmp") && rm "$tmp" ;
else
cd "$(\lf -print-last-dir "$@")"
fi
}
credit again to @3ximus for the help ๐.
After doing some more research, I have found that tmux
uses the CWD of the foreground group leader, otherwise session leader as pane_current_path
, see https://github.com/tmux/tmux/blob/608d113486835515e7a89b1511704440c68ae817/osdep-linux.c#L63-L89.
This is a somewhat hacky heuristic that isn't necessarily correct in all cases, and as a result tmux
also provides pane_path
, which can be set by applications using the OSC 7 terminal sequence. I think this is a better solution than changing the implementation of lfcd
.
lfrc
:
# set pane_path when changing directory
cmd on-cd &{{
printf "\e]7;$PWD\e\\" > /dev/tty
}}
# unset pane_path when quitting
cmd on-quit &{{
printf "\e]7;\e\\" > /dev/tty
}}
# trigger on-cd upon startup
on-cd
tmux.conf
:
# use pane_path, falling back to pane_current_path
bind v split-window -h -c "#{?pane_path,#{pane_path},#{pane_current_path}}"
bind s split-window -v -c "#{?pane_path,#{pane_path},#{pane_current_path}}"
Yes, this works well. I've been using it this way for the last four days now, and no problems.
Where do you think is the best place for this solution to live? I could see it going well as part of the lfrc.example
or as a comment in lfcd.sh
.
Works for me! Thanks for the help. I changed the issue title to make it easier to find if someone else runs into this.