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

promptd-vcs colors cause display issues in bash #4

Closed
joelnordell opened this issue Mar 10, 2015 · 12 comments
Closed

promptd-vcs colors cause display issues in bash #4

joelnordell opened this issue Mar 10, 2015 · 12 comments
Assignees
Labels

Comments

@joelnordell
Copy link
Contributor

Bash uses the character length of the prompt (vs. terminal width) to decide whether to line-wrap the input. The color codes in the promptd-vcs output appear to confuse bash, causing it to line-wrap too soon. Perhaps it could escape the output in a way for bash to interpret?

See this discussion for example, which might shed some light on what would need to be done:
http://stackoverflow.com/questions/342093/ps1-line-wrapping-with-colours-problem

@mrkline
Copy link
Owner

mrkline commented Mar 10, 2015

Thanks for catching this! I ran into a similar issue in Zsh where color codes need to be escaped (see the --zsh option). I'll do something similar for bash when I get home from work. Or perhaps using tput might be advantageous.

@mrkline mrkline self-assigned this Mar 10, 2015
@mrkline mrkline added the bug label Mar 10, 2015
mrkline added a commit that referenced this issue Mar 11, 2015
@mrkline
Copy link
Owner

mrkline commented Mar 11, 2015

@joelnordell, does that look like what you had in mind?

@joelnordell
Copy link
Contributor Author

Thanks for the quick work on that! Unfortunately, it didn't work (for me). I did some investigating, and attempted various modifications to your code (including outputting un-escaped [ characters, and replacing the ESC codes with literal \033 for bash to interpret for itself), and as far as I can tell bash doesn't honor the \[ escaping when it is part of the output of an external command. Seems to be a fundamental flaw in bash.

Try this example to illustrate what I mean:

  1. Create a text file, named /tmp/test.txt, containing an escaped ANSI sequence:

    echo -e "\[\033[01;34m\]test\[\033[00m\] " > /tmp/test.txt
  2. Set PS1 using the contents of the file directly.

    PS1=cat /tmp/test.txt``
  3. Set PS1 using a subshell to call cat to include the file (similar to including promptd-vcs).

    PS1='$(cat /tmp/test.txt)'

Note that the prompt works properly in step 2 but does not work in step 3. Not sure if there's any way around this short of fixing bash itself, unfortunately! :( I wonder if a newer version of bash fixes this?

@joelnordell
Copy link
Contributor Author

I guess I should've verified bash's ability to make use of those escapes from an external program before filing the bug report 😜

@mrkline
Copy link
Owner

mrkline commented Mar 11, 2015

How strange. I guess I have to do some more reading on Bash tonight.

@mrkline
Copy link
Owner

mrkline commented Mar 16, 2015

It looks like http://stackoverflow.com/a/24840720/713961 might be our solution. I'll take a crack at that tonight.

@joelnordell
Copy link
Contributor Author

Ah, good find. In particular this comment might hold the key:

You're entirely right. \001 and \002, aka RL_PROMPT_START_IGNORE and RL_PROMPT_END_IGNORE, are a poorly documented readline feature. Bash converts [..] to \001..\002 for Readline, but neglects to escape existing ones so that this implementation detail leaks through (and it's so useful that you can hardly call it a bug).

I'll give that a quick try and report back.

@joelnordell
Copy link
Contributor Author

I made this change in color.d and it works!

@@ -43,5 +43,5 @@ string zshEscape(string code)

 string bashEscape(string code)
 {
-       return `\[` ~ code ~ `\]`;
+       return "\001" ~ code ~ "\002";
 }

@mrkline
Copy link
Owner

mrkline commented Mar 16, 2015

Fantastic! I can fix that up this evening, or if you'd like to open a pull request, I'd be happy to merge it.

@joelnordell
Copy link
Contributor Author

Ok, done -- #8

@mrkline mrkline closed this as completed Mar 16, 2015
@mrkline
Copy link
Owner

mrkline commented Mar 16, 2015

Thanks much! I'll tag a release with it shortly, FWIW.

@joelnordell
Copy link
Contributor Author

Thank you, too, for researching this & finding the actual solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants