Skip to content
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

Use 'cat' binary to restore pane contents rather than (potentially) using a shell command or alias #313

Closed
wants to merge 1 commit into from

Conversation

9999years
Copy link

pane_creation_command in scripts/restore.sh uses the string cat to run /bin/cat; however, because pane_creation_command is evaluated in the shell, aliases of cat may cause unexpected results; in my case, it runs the bat binary, which produces weird and garbled output. By using command -v, we can get the path of the cat binary and avoid this. (Note that we don't use which because which isn't POSIX-standard.)

`pane_creation_command` in `scripts/restore.sh` uses the string `cat` to
run `/bin/cat`; however, because `pane_creation_command` is evaluated in
the shell, aliases of `cat` may cause unexpected results; in my case, it
runs the `bat` [1] binary, which produces weird and garbled output. By
using `command -v`, we can get the path of the `cat` binary and avoid
this. (Note that we don't use `which` because `which` isn't
POSIX-standard.)

[1]: https://github.com/sharkdp/bat
@9999years 9999years changed the title Use 'cat' command always Use 'cat' binary to restore pane contents rather than (potentially) using a shell command or alias Oct 19, 2019
@bruno-
Copy link
Member

bruno- commented Oct 20, 2019

Hi,

I think overriding standard shell commands is an anti-pattern.

I'll leave this PR open in case the community disagrees, but I'm not in favor of merging this.

@9999years
Copy link
Author

Even if redefining coreutils commands is an anti-pattern, running a binary using its absolute name is the correct way to send commands to a user's shell; given that this patch has no cost — it makes tmux-resurrect work on more systems without doing anything unclear or strange in the code — I think we should merge it.

@eproxus
Copy link

eproxus commented Mar 23, 2021

I have a gut feeling this will solve #192, or at least is related to it. When using Fish as a shell I always get this message in the last pane and no history:

cat: /Users/user/.tmux/resurrect/restore/pane_contents//pane-dev:3.1: No such file or directory

@eproxus
Copy link

eproxus commented Mar 23, 2021

Just tested the patch and it seems it did not fix #192.

@eproxus
Copy link

eproxus commented May 31, 2021

Just hit this problem again today. Using Fish shell and setting an alias from cat to bat breaks the pane restoration. Is there any possibility to merge this? I assume lots of people could have cat aliases.

@9999years 9999years closed this Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants