gokcehan/lf

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

  1. open tmux
  2. run lf
  3. navigate to any folder
  4. 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

  1. open tmux
  2. this time, run cd "$(command \lf -print-last-dir "$@")"
  3. navigate to any folder
  4. 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 evaluate pane_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 in tmux or not
  • Adding the mktemp implementation to lfcd.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.

This belongs in the wiki, which contains configuration snippets submitted by users for various functionalities.

I have already added the above configuration as an example, hopefully this is OK for you.

Works for me! Thanks for the help. I changed the issue title to make it easier to find if someone else runs into this.