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

iTerm2 LP_ENABLE_TITLE not escaping properly #581

Closed
kapad opened this issue Aug 12, 2019 · 20 comments
Closed

iTerm2 LP_ENABLE_TITLE not escaping properly #581

kapad opened this issue Aug 12, 2019 · 20 comments
Milestone

Comments

@kapad
Copy link

kapad commented Aug 12, 2019

I'm using iTerm2 and when I set LP_ENABLE_TITLE=1 but the title doesn't seem to be escaped correctly, and I see ?? at the end for every terminal.

What would be the correct settings for LP_TITLE_OPEN and LP_TITLE_CLOSE to fix this?

Shell: bash
Operating system: macOS Mojave
Liquid Prompt version (tag, commit): 1.11 2016-06-25 (Installed from Homebrew).

@kapad kapad changed the title Mac LP_ENABLE_TITLE iTerm2 LP_ENABLE_TITLE not escaping properly Aug 12, 2019
@Rycieos
Copy link
Collaborator

Rycieos commented Aug 16, 2019

Note that liquidprompt version 1.11 is very out of date, and reported issues are only supported on the latest version.

However, I am also seeing this issue. I'm on iTerm2 v3.3.1 and latest liquidprompt (eda83ef). I have a mostly stock liquidpromptrc, and I can't see anything obvious that would cause this. I'll keep digging.

@Rycieos
Copy link
Collaborator

Rycieos commented Oct 22, 2020

I no longer have access to a Mac to test/develop a fix for this. I still want this to make v2.0, but I'll need help.

@Rycieos Rycieos added the help wanted An issue where ideas or patches are welcome label Oct 22, 2020
@kapad
Copy link
Author

kapad commented Oct 22, 2020

I've got a Mac and am happy to test and even develop/bug fix. I've stopped using liquidprompt right now though, so which version should I download to test this? Has any fix been attempted or am I going to be testing if the bug is still around?

@Rycieos
Copy link
Collaborator

Rycieos commented Oct 22, 2020

Thanks for being willing to help even if you don't use it.

Version v1.12.0 is our latest stable, you should test with that. I think a few things got changed around terminal titles, but I don't really expect the issue to be gone.

I do plan on doing some work on the default terminal title, but I would want to know what is causing this issue so we can avoid the problem for the future implementation.

@kapad
Copy link
Author

kapad commented Oct 22, 2020 via email

@Rycieos Rycieos added the needs information More information needed from the reporter label Nov 3, 2020
@kapad
Copy link
Author

kapad commented Nov 8, 2020

image

I'm seeing that in my iTerm2 title.

Steps that I've taken to test:

  • Created a new profile in iTerm2 to load a zsh shell without any RC files.
  • executed export LP_ENABLE_TITLE=1
  • executed source ~/liquidprompt/liquidprompt
  • My terminal title changed to that.. I'm not sure where the ^M^J is coming from.

I was using the v1.12.1 version.

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 8, 2020

Perfect. Could you run printf '%q\n' "$PS1" and post the output?

And if you wouldn't mind doing the same thing (with clean rc files) for Bash, just so we can rule out shell issues.

@kapad
Copy link
Author

kapad commented Nov 8, 2020 via email

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 8, 2020

bash --noprofile --norc should do it.

If it's inheriting from the parent shell, try env -i bash --norc --noprofile

@kapad
Copy link
Author

kapad commented Nov 9, 2020

Steps followed

  • env -i bash --norc --noprofile
  • export LP_ENABLE_TITLE=1
  • source ~/liquidprompt/liquidprompt

Result
No change to my prompt. It's like liquidprompt never loaded. Is this some regression?

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 9, 2020

Hm, unlikely. We have checks to not load if not in an interactive terminal. Probably loading bash like that is not loading any env vars at all.

Try export PS1="$ " and export TERM=xterm-256color before loading Liquidprompt.

Also Liquidprompt will still load its config, so of you have title disabled in the config, that export before loading Liquidprompt won't do anything. It would only do something if you don't have $LP_ENABLE_TITLE set at all in the config.

@kapad
Copy link
Author

kapad commented Nov 9, 2020

I just created a different profile in iTerm for this. :)

And the issue is the same as when I tested with zsh. There's an extra ^M^J in the title.
image

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 9, 2020

Glad to see that, should make debugging easier.

Can you post the output of printf '%q\n' "$PS1"?

@kapad
Copy link
Author

kapad commented Nov 12, 2020

Output of printf '%q\n' "$PS1"

Bash

$'\\[\E]0;⌁58% [\\u:~] \\$ \n\a\\]\\[\E[31m\\]⌁\\[\E(B\E[m\\]\\[\E[33m\\]58%\\[\E(B\E[m\\] [\\[\E[1m\\]\\u\\[\E(B\E[m\\]\\[\E[32m\\]:\\[\E(B\E[m\\]\\[\E[1m\\]~\\[\E(B\E[m\\]] \\[\E[1m\\]\\$\\[\E(B\E[m\\] '

zsh

%\{$'\033'\]0\;⌁57%%\ \[%n:~\]\ %\(\!.\#.%%\)\ $'\n'$'\a'%\}%\{$'\033'\[31m%\}⌁%\{$'\033'\(B$'\033'\[m%\}%\{$'\033'\[33m%\}57%%%\{$'\033'\(B$'\033'\[m%\}\ \[%\{$'\033'\[1m%\}%n%\{$'\033'\(B$'\033'\[m%\}%\{$'\033'\[32m%\}:%\{$'\033'\(B$'\033'\[m%\}%\{$'\033'\[1m%\}~%\{$'\033'\(B$'\033'\[m%\}\]\ %\{$'\033'\[1m%\}%\(\!.\#.%%\)%\{$'\033'\(B$'\033'\[m%\}\

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 12, 2020

Break it down:
\\[\E]0;⌁58% [\\u:~] \\$ \n\a\\]
remove duplicate \:
\[\E]0;⌁58% [\u:~] \$ \n\a\]
remove Bash nonprinting markers (\[ \]):
\E]0;⌁58% [\u:~] \$ \n\a
remove title start and end markers (\E]0; \a):
⌁58% [\u:~] \$ \n

Oh, yeah, that's obvious. Both have a \n sequence at the end of the title. ^J is the caret notation for the newline control character. I guess my terminal emulator (Mintty) is lacking by not displaying it when I test with your PS1. So iTerm2 is working (partially) correctly displaying the character we send it. I guess it is converting the \n sequence into a \r\n sequence instead, then displaying that.

So why does the title have that character, but the main prompt doesn't? My only guess is that this line:
https://github.com/nojhan/liquidprompt/blob/ec891eaada5cd427292055603c7dbe977d478e67/liquidprompt#L1594
the sed command is somehow returning a \n only on MacOS...

Coincidentally I've been working on a way to replace control chars in the prompt with #632. I'm not sure if that would help here since 1) this doesn't seem to be originating from a data function and 2) it is in the title, but not the main prompt.

What I need from you

Could you test this command in Bash for me?
printf '%q\n' "$(_lp_as_text $'\\[\E[31m\\]text\\[\E(B\E[m\\]'; printf x)"

Are you using a custom .ps1 file? If so, can you add it here?

What version of Liquidprompt are you running now?

@kapad
Copy link
Author

kapad commented Nov 12, 2020

For all the tests that I ran, I'd pinned the version to v1.12.1. Should I change it for any further test?
Nope, I'm not using any custom .ps1 for liquidprompt. I used to at the time that liquid was my primary prompt, but, for these tests, I simply sourced the liquidprompt script.

the sed command is somehow returning a \n only on MacOS...

This could very well be the culprit. Since this was testing, I'd created new profiles for it in iTerm, but, these profiles didn't have any PATH setup, so the sed that would have been invoked would have been MacOS's sed. Since for work I need compatibility between Mac and Linux, I use the GNU binary for sed from Homebrew.
I'll check this again, but setup the PATH to be identical to my what I've got for work. This'll also take care of awk and grep. (GNU versions only).

printf '%q\n' "$(_lp_as_text $'\\[\E[31m\\]text\\[\E(B\E[m\\]'; printf x)"

And I'll also give you the output to this in bash.

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 12, 2020

I'd pinned the version to v1.12.1. Should I change it for any further test?

No, that is the latest stable, so perfect. I just wanted to make sure since you aren't actively using Liquidprompt you weren't on an older version.

@kapad
Copy link
Author

kapad commented Nov 17, 2020

Could you test this command in Bash for me?
printf '%q\n' "$(_lp_as_text $'\\[\E[31m\\]text\\[\E(B\E[m\\]'; printf x)"

Output
⌁54% [kapad:~] $ printf '%q\n' "$(_lp_as_text $'\\[\E[31m\\]text\\[\E(B\E[m\\]'; printf x)"
$'text\nx'

@Rycieos
Copy link
Collaborator

Rycieos commented Nov 17, 2020

Thanks @kapad, you have been very helpful tracking this down.

It turns out that sed will add a \n on the last line on MacOS. This is apparently a well documented "issue":
https://stackoverflow.com/q/10226855/8338372
https://stackoverflow.com/q/13325138/8338372

All of our other uses of sed in Liquidprompt don't have this issue as they are wrapped in $(...), which by design strips trailing newline characters. This one is too (as a wrapper around _lp_title()), but we echo stuff after it:
https://github.com/nojhan/liquidprompt/blob/ec891eaada5cd427292055603c7dbe977d478e67/liquidprompt#L1601-L1604

While working on v2.0, I redesigned some of the title internals, which actually accidentally fixed this problem.

I'm hesitant to fix this in a v1.12.2, as it's already fixed in v2.0 (which is hopefully getting close to release), and I don't see any way to fix it before v2.0 without adding another subshell, adding extra runtime for all users.

I'll mark this as will be fixed by v2.0, and update this issue when that happens.

@Rycieos
Copy link
Collaborator

Rycieos commented Dec 9, 2020

As I mentioned before, I accidentally fixed this. It is now fixed in v2.0.0-beta.1. However, if you are using a custom .ps1 file, and were calling _lp_title() in it, you will need to follow the upgrade docs to replace that call with _lp_formatted_title().

@Rycieos Rycieos closed this as completed Dec 9, 2020
@Rycieos Rycieos removed help wanted An issue where ideas or patches are welcome needs information More information needed from the reporter labels Dec 9, 2020
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

No branches or pull requests

2 participants