[Home]

Summary:ASTERISK-25587: Libedit2 colored prompt is broken beyond repair
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2015-11-22 09:20:46.000-0600Date Closed:
Priority:MinorRegression?No
Status:Open/NewComponents:Core/General
Versions:11.20.0 13.6.0 Frequency of
Occurrence
Related
Issues:
is related toASTERISK-25695 safe_asterisk -c breaks color in asterisk -r because of missing TERM for /dev/tty.
is related toASTERISK-25585 [patch]rasterisk never hits most of main(), but it's assumed to
Environment:Attachments:
Description:Readline/bash uses {{\\\[}} and {{\\\]}} to delimit portions of the shell prompt that denote color (and do not move the cursor).

For example:
{noformat}
PS1="`printf '\\[\033[32;1m\\]Prompt\\[\033[0m\\]$ '`"
{noformat}
Yields a colored prompt with "Prompt" in green.
{noformat}
Prompt$
{noformat}
The colorization bits aren't counted, and when you browse through history or move back from the next line, the prompt stays where it should be.

If you omit the {{\\\[}} and the {{\\\]}}, the prompt behaves oddly when browsing the history or moving back from the next line:
{noformat}
PS1="`printf '\033[32;1mPrompt\033[0m$ '`"
{noformat}
Now, after arrow-up, arrow-down, the prompt looks like this:
{noformat}
Prompt$ PS1="`print<--- start of the prompt
{noformat}

Asterisk also allows one to colorize the prompt, but there is no working escape to not count the colorization bits:
{noformat}
ASTERISK_PROMPT='%C32Prompt%C5$ ' ./main/asterisk -c
{noformat}

That works, and gets you a green prompt. But it suffers from the readline problem as mentioned above (when you omit the {{\\\[}} and {{\\\]}}).

Libedit2 claims to have a solution to that, by surrounding the colorization bits with {\\\1}}, like this. (Define color_start and color_end as {{"\\\1"}}.)
{noformat}
@@ -2625,10 +2633,16 @@ static char *cli_prompt(EditLine *editline)
                               case 'C': /* color */
                                       t++;
                                       if (sscanf(t, "%30d;%30d%n", &fgcolor, &bgcolor, &i) == 2) {
-                                               ast_str_append(&prompt, 0, "%s", term_color_code(term_code, fgcolor, bgc
+                                               ast_str_append(&prompt, 0, "%s%s%s",
+                                                       color_start,
+                                                       term_color_code(term_code, fgcolor, bgcolor, sizeof(term_code)),
+                                                       color_end);
                                               t += i - 1;
                                       } else if (sscanf(t, "%30d%n", &fgcolor, &i) == 1) {
-                                               ast_str_append(&prompt, 0, "%s", term_color_code(term_code, fgcolor, 0,
+                                               ast_str_append(&prompt, 0, "%s%s%s",
+                                                       color_start,
+                                                       term_color_code(term_code, fgcolor, 0, sizeof(term_code)),
+                                                       color_end);
                                               t += i - 1;
                                       }

{noformat}
and this:
{noformat}
@@ -2979,7 +3001,11 @@ static int ast_el_initialize(void)
               history_end(el_hist);

       el = el_init("asterisk", stdin, stdout, stderr);
+#ifdef EL_PROMPT_ESC
+       el_set(el, EL_PROMPT_ESC, cli_prompt, '\1');
+#else
       el_set(el, EL_PROMPT, cli_prompt);
+#endif

       el_set(el, EL_EDITMODE, 1);
       el_set(el, EL_EDITOR, editor);
{noformat}

However, the above only works if you define *only* one color and no color reset because of this bug:
http://gnats.netbsd.org/47539

_The color sequence is output out of band of the non-color, so all the colors strings (both the set and the reset) end up before the the prompt is printed: the prompt gets no color. (This was confirmed with the libedit-3.1-20130712 version from Ubuntu Trusty.)_

This is not easily fixable within libedit2 because of how the virtual buffer works. (Perhaps it's fixed somewhere, I didn't find it easily.)

To make matters worse, the color reset in Asterisk is also broken:
{noformat}
               if (color_used) {
                       /* Force colors back to normal at end */
                       ast_str_append(&prompt, 0, "%s", term_color_code(term_code, 0, 0, sizeof(term_code)));
               }
{noformat}
That bit does not work, because fgcolor is 0, and when it is 0, we end up here. (Read as: the patch fixes that problem.)
{noformat}
@@ -265,7 +265,7 @@ char *term_color_code(char *outbuf, int fgcolor, int bgcolor, int maxout)
{
       int attr = 0;

-       if (!check_colors_allowed(fgcolor)) {
+       if (!check_colors_allowed()) {
               *outbuf = '\0';
               return outbuf;
       }
...
@@ -234,16 +234,16 @@ static void check_bgcolor(int *bgcolor)
       }
}

-static int check_colors_allowed(int fgcolor)
+static int check_colors_allowed(void)
{
-       return (!vt100compat || !fgcolor) ? 0 : 1;
+       return vt100compat;
}

{noformat}

On top of all the above, the colored prompt has been broken in rasterisk (-r) for quite some time -- I believe, see ASTERISK-25585. So if we dropped support for it, no one would notice.

Suggested fix: *drop color support*

Something like this:
{noformat}
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -2625,10 +2625,8 @@ static char *cli_prompt(EditLine *editline)
                               case 'C': /* color */
                                       t++;
                                       if (sscanf(t, "%30d;%30d%n", &fgcolor, &bgcolor, &i) == 2) {
-                                               ast_str_append(&prompt, 0, "%s", term_color_code(term_code, fgcolor, bgc
                                               t += i - 1;
                                       } else if (sscanf(t, "%30d%n", &fgcolor, &i) == 1) {
-                                               ast_str_append(&prompt, 0, "%s", term_color_code(term_code, fgcolor, 0,
                                               t += i - 1;
                                       }

{noformat}

(Sorry for the inline patch, I hereby release all my copyright claims, like usual.)

(Argh, JIRA refuses to give a literal BASKSLASH-BRACKET. Only three backslashes work, but when it displays all three... Gah.)
Comments:By: Asterisk Team (asteriskteam) 2015-11-22 09:20:48.800-0600

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].
By: Matt Jordan (mjordan) 2015-11-22 22:54:30.638-0600

I would love to drop the coloured prompt support.

I attempted during Asterisk 13 (when it was 'trunk' in SVN) to try and get the colour support to suck ... less. We had a lot of weird things where the 'Asterisk Ready' prompt wouldn't show up on a white background, or would show up as black text on a black background. I don't think I made much of an improvement, but it is (maybe) a bit better.

One place I struck out on was the colour prompt. I never did file an issue to fix it - frankly, you took the investigation a whole lot farther than I ever got with it.

Since this is one of those barely documented features that I'm not sure ever fully worked, I'd be all for removing it.
By: Rusty Newton (rnewton) 2015-11-23 16:36:27.825-0600

bq. Since this is one of those barely documented features that I'm not sure ever fully worked, I'd be all for removing it.

+1