[asterisk-bugs] [JIRA] (ASTERISK-25587) Libedit2 colored prompt is broken beyond repair

[Walter Doekes (JIRA)]

Walter Doekes created ASTERISK-25587:
----------------------------------------

             Summary: Libedit2 colored prompt is broken beyond repair
                 Key: ASTERISK-25587
                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-25587
             Project: Asterisk
          Issue Type: Bug
      Security Level: None
    Affects Versions: 13.6.0, 11.20.0
            Reporter: Walter Doekes
            Severity: Minor


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.)



--
This message was sent by Atlassian JIRA
(v6.2#6252)