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

Add nested tmux escape sequence for bash prompt tracking #552

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

d-p
Copy link

@d-p d-p commented Nov 2, 2021

The single tmux instances are unaffected by the change.

The implementation uses the single quote to simplify the escaping of
backslashes (avoid doubling them) and replaces the octal value of the
bell sequence (\007) with the corresponding C escape sequence (\a) for
uniformity with ESC (\033 or \e).

The single tmux instances are unaffected by the change.

The implementation uses the single quote to simplify the escaping of
backslashes (avoid doubling them) and replaces the octal value of the
bell sequence (\007) with the corresponding C escape sequence (\a) for
uniformity with ESC (\033 or \e).
@Sbozzolo
Copy link
Collaborator

Sbozzolo commented Nov 2, 2021

Thanks for your PR. Is there a reason to prefer the "simple" method as opposed to the one that works for nested sessions as well?

@d-p
Copy link
Author

d-p commented Nov 2, 2021

Thanks for your PR. Is there a reason to prefer the "simple" method as opposed to the one that works for nested sessions as well?

Thanks for the feedback.

You it the pain point... I do not know why the nested escape also works in the simple session (I was surprised!). I hope that this is by the desing of tmux and not by an accident (tested in tmux 3.1c). I left the simple implementation as documentation/workaround if some tmux version/build will raise warning or errors.

By looking at tmux source code (quickly) I did not found any relevant test/regression about nested escapes. If you prefer to be safe than sorry we need to check/add proper escape sequence testing in tmux. At this time I do not know if this is worth the effort and in the scope here. What do you think?

@Sbozzolo
Copy link
Collaborator

Sbozzolo commented Nov 3, 2021

I don't know much about tmux, but we should definitely ensure that the same behavior is available for zsh and fish as well (ie, we should also edit the corresponding files). If everything is consistent, we can merge this, leaving a descriptive comment. The comment should explain why one might want to use the simple version (which is more or less what you wrote in the last message).

Thanks.

@d-p
Copy link
Author

d-p commented Nov 5, 2021

Thanks for the quick feedback loop, I will try to add some tests and then add/refactor the shell code.
Probably the tests will be another pull request (hopefully in the near future) and then I will update this one.

@jixiuf jixiuf force-pushed the master branch 2 times, most recently from 9a6dbeb to a2f2286 Compare April 14, 2022 08:34
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.

2 participants