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

From: Roberto E. Vargas Caballero <k0ga_AT_shike2.com>
Date: Mon, 21 Mar 2022 09:24:50 +0100

Hi,

A few small nitpicks about formating (fell free to ignore them
if you want ;)):

On Sun, Mar 20, 2022 at 06:25:40PM +0600, NRK wrote:
> _AT_@ -1843,39 +1844,25 @@ 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);

I think is better to keep every ternary in a line by itself, because it makes
easier to read them:

                fprintf(stderr, "erresc: failed to fetch %s color %d\n",
                        is_osc4 ? "osc4" : "osc",
                        is_osc4 ? num : index);


> + 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);
> + if (n < 0 || n >= sizeof(buf))
> + fprintf(stderr, "error: %s while printing %s response\n", n < 0 ?
> + "snprintf failed" : "truncation occurred", is_osc4 ? "osc4" : "osc");
> + else
> + ttywrite(buf, n, 1);
> }


I think we force here to put braces around if and else because the body of the
if part is more of one line. Again, I think is better to use a line for every
ternary and have something like:


        if (n < 0 || n >= sizeof(buf)) {
                fprintf(stderr, "error: %s while printing %s response\n",
                        n < 0 ? "snprintf failed" : "truncation occurred",
                        is_osc4 ? "osc4" : "osc");
        } else {
                ttywrite(buf, n, 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;

Same apply here, I think our style forces to have braces in every if and else if
because there is a body with more of one lines.


Regards,
Received on Mon Mar 21 2022 - 09:24:50 CET