Re: [hackers] [st][PATCH v2] code-golfing: cleanup osc color related code

From: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
Date: Sun, 20 Mar 2022 12:49:22 +0100

On Sun, Mar 20, 2022 at 05:36:04PM +0600, NRK wrote:
> * adds missing function prototype
> * move xgetcolor() prototype to win.h (that's where all the other x.c
> func prototype seems to be declared at).
> * reduces code duplication for osc 10/11/12
> * unify osc_color_response() and osc4_color_response() into a single function.
>
> the latter two was suggested by Quentin Rameau in his patch review on
> the hackers list.
> ---
> st.c | 72 ++++++++++++++++-------------------------------------------
> st.h | 2 --
> win.h | 1 +
> 3 files changed, 20 insertions(+), 55 deletions(-)
>
> diff --git a/st.c b/st.c
> index c71fa06..068c92c 100644
> --- a/st.c
> +++ b/st.c
> _AT_@ -161,6 +161,7 @@ static void csidump(void);
> static void csihandle(void);
> static void csiparse(void);
> static void csireset(void);
> +static void osc_color_response(int, int, int);
> static int eschandle(uchar);
> static void strdump(void);
> static void strhandle(void);
> _AT_@ -1843,38 +1844,20 @@ csireset(void)
> }
>
> void
> -osc4_color_response(int num)
> +osc_color_response(int num, int index, int is_osc4)
> {
> int n;
> char buf[32];
> unsigned char r, g, b;
>
> - if (xgetcolor(num, &r, &g, &b)) {
> - fprintf(stderr, "erresc: failed to fetch osc4 color %d\n", num);
> + if (xgetcolor(is_osc4 ? num : index, &r, &g, &b)) {
> + fprintf(stderr, "erresc: failed to fetch %s color %d\n",
> + is_osc4 ? "osc4" : "osc", is_osc4 ? num : index);
> return;
> }
>
> - n = snprintf(buf, sizeof buf, "\033]4;%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
> - num, r, r, g, g, b, b);
> -
> - ttywrite(buf, n, 1);
> -}

A nitpick, because I think the string always fits in the buffer `buf`, but: the
return value can be negative or on truncation >= sizeof(buf). So the pattern
of:

        n = snprintf(...);
        ttywrite(..., n, ...);

Is not good.

> -
> -void
> -osc_color_response(int index, int num)
> -{
> - int n;
> - char buf[32];
> - unsigned char r, g, b;
> -
> - if (xgetcolor(index, &r, &g, &b)) {
> - fprintf(stderr, "erresc: failed to fetch osc color %d\n", index);
> - return;
> - }
> -
> - n = snprintf(buf, sizeof buf, "\033]%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
> - num, r, r, g, g, b, b);
> -
> + n = snprintf(buf, sizeof buf, "\033]%s%d;rgb:%02x%02x/%02x%02x/%02x%02x\007",
> + is_osc4 ? "4;" : "", num, r, r, g, g, b, b);
> ttywrite(buf, n, 1);
> }
>
> _AT_@ -1883,6 +1866,11 @@ strhandle(void)
> {
> char *p = NULL, *dec;
> int j, narg, par;
> + const struct { int idx; char *str; } osc_table[] = {
> + { defaultfg, "foreground" },
> + { defaultbg, "background" },
> + { defaultcs, "cursor" }
> + };
>
> term.esc &= ~(ESC_STR_END|ESC_STR);
> strparse();
> _AT_@ -1917,41 +1905,19 @@ strhandle(void)
> }
> return;
> case 10:
> - if (narg < 2)
> - break;
> -
> - p = strescseq.args[1];
> -
> - if (!strcmp(p, "?"))
> - osc_color_response(defaultfg, 10);
> - else if (xsetcolorname(defaultfg, p))
> - fprintf(stderr, "erresc: invalid foreground color: %s\n", p);
> - else
> - tfulldirt();
> - return;
> case 11:
> - if (narg < 2)
> - break;
> -
> - p = strescseq.args[1];
> -
> - if (!strcmp(p, "?"))
> - osc_color_response(defaultbg, 11);
> - else if (xsetcolorname(defaultbg, p))
> - fprintf(stderr, "erresc: invalid background color: %s\n", p);
> - else
> - tfulldirt();
> - return;
> case 12:
> if (narg < 2)
> break;
> -
> p = strescseq.args[1];
> + if ((j = par - 10) < 0 || j >= LEN(osc_table))
> + break; /* shouldn't be possible */
>
> if (!strcmp(p, "?"))
> - osc_color_response(defaultcs, 12);
> - else if (xsetcolorname(defaultcs, p))
> - fprintf(stderr, "erresc: invalid cursor color: %s\n", p);
> + osc_color_response(par, osc_table[j].idx, 0);
> + else if (xsetcolorname(osc_table[j].idx, p))
> + fprintf(stderr, "erresc: invalid %s color: %s\n",
> + osc_table[j].str, p);
> else
> tfulldirt();
> return;
> _AT_@ -1964,7 +1930,7 @@ strhandle(void)
> j = (narg > 1) ? atoi(strescseq.args[1]) : -1;
>
> if (p && !strcmp(p, "?"))
> - osc4_color_response(j);
> + osc_color_response(j, 0, 1);
> else if (xsetcolorname(j, p)) {
> if (par == 104 && narg <= 1)
> return; /* color reset without parameter */
> diff --git a/st.h b/st.h
> index 519b9bd..fd3b0d8 100644
> --- a/st.h
> +++ b/st.h
> _AT_@ -111,8 +111,6 @@ void *xmalloc(size_t);
> void *xrealloc(void *, size_t);
> char *xstrdup(const char *);
>
> -int xgetcolor(int x, unsigned char *r, unsigned char *g, unsigned char *b);
> -
> /* config.h globals */
> extern char *utmp;
> extern char *scroll;
> diff --git a/win.h b/win.h
> index e6e4369..6de960d 100644
> --- a/win.h
> +++ b/win.h
> _AT_@ -30,6 +30,7 @@ void xdrawline(Line, int, int, int);
> void xfinishdraw(void);
> void xloadcols(void);
> int xsetcolorname(int, const char *);
> +int xgetcolor(int, unsigned char *, unsigned char *, unsigned char *);
> void xseticontitle(char *);
> void xsettitle(char *);
> int xsetcursor(int);
> --
> 2.34.1
>
>

-- 
Kind regards,
Hiltjo
Received on Sun Mar 20 2022 - 12:49:22 CET

This archive was generated by hypermail 2.3.0 : Sun Mar 20 2022 - 13:00:37 CET