csi: fix sub-parameter versions of 38/48 SGR escapes

Well this is embarrassing; the sub-parameter versions of the 38/48 SGR
escapes all required an extra ‘:2’ that wasn’t supposed to be there,
causing all the other sub-parameters to be shifted one step to the
right.

That is, foot expected e.g. 38:2:2:r:g:b, or 38:2:5:idx when the
correct sequences are 38:2:cs:r:g:b and 38:5:idx.

I.e. I mixed up the color-space ID (cs) of 38:2 with *type* of color:
RGB or indexed.

In addition to fixing this, this patch also adds support for a
“bastard” version of the sub-parameter based RGB escapes, where the
color-space identifier has been left out: e.g. 38:2:r:g:b. This
sequence is invalid, but applications tend to “forget” the color-space
ID...

CHANGELOG.md

@@ -107,6 +107,10 @@ means foot can be PGO:d in e.g. sandboxed build scripts. See
   has been used to enable blinking. **cursor.blink** in `foot.ini`
   controls the default state of `CSI Ps SP q`
   (https://codeberg.org/dnkl/foot/issues/218).
+* The sub-parameter versions of the SGR RGB color escapes (e.g
+  `\E[38:2...m`) can now be used _without_ the color space ID
+  parameter.
+
 
 ### Deprecated
 ### Removed
@@ -125,6 +129,8 @@ means foot can be PGO:d in e.g. sandboxed build scripts. See
   attributes and charset configuration, just like `\E7`+`\E8`.
 * Report mouse motion events to the client application also while
   dragging the cursor outside the grid.
+* Parsing of the sub-parameter versions of indexed SGR color escapes
+  (e.g. `\E[38:5...m`)
 
 
 ### Security

csi.c

@@ -141,53 +141,51 @@ csi_sgr(struct terminal *term)
                 i += 4;
             }
 
-            /* Sub-parameter style: 38:2:... */
+            /* Indexed: 38:5:<idx> */
             else if (term->vt.params.v[i].sub.idx >= 2 &&
+                     term->vt.params.v[i].sub.value[0] == 5)
+            {
+                const struct vt_param *param = &term->vt.params.v[i];
+
+                uint8_t idx = param->sub.value[1];
+                term->vt.attrs.have_fg = 1;
+                term->vt.attrs.fg = term->colors.table[idx];
+            }
+
+            /*
+             * RGB: 38:2:<color-space>:r:g:b[:ignored:tolerance:tolerance-color-space]
+             * RGB: 38:2:r:g:b
+             *
+             * The second version is a "bastard" version - many
+             * programs "forget" the color space ID
+             * parameter... *sigh*
+             */
+            else if (term->vt.params.v[i].sub.idx >= 4 &&
                      term->vt.params.v[i].sub.value[0] == 2)
             {
                 const struct vt_param *param = &term->vt.params.v[i];
-                const int color_space_id = param->sub.value[1];
+                bool have_color_space_id = param->sub.idx >= 5;
 
-                switch (color_space_id) {
+                /* 0 - color space (ignored) */
-                case 0:   /* Implementation defined - we map it to '2' */
+                int r_idx = 2 - !have_color_space_id;
-                case 2: { /* RGB - 38:2:2:<r>:<g>:<b> */
+                int g_idx = 3 - !have_color_space_id;
-                    if (param->sub.idx < 5) {
+                int b_idx = 4 - !have_color_space_id;
-                        UNHANDLED_SGR(i);
+                /* 5 - unused */
-                        break;
+                /* 6 - CS tolerance */
-                    }
+                /* 7 - color space associated with tolerance */
 
-                    uint8_t r = param->sub.value[2];
+                uint8_t r = param->sub.value[r_idx];
-                    uint8_t g = param->sub.value[3];
+                uint8_t g = param->sub.value[g_idx];
-                    uint8_t b = param->sub.value[4];
+                uint8_t b = param->sub.value[b_idx];
-                    /* 5 - unused */
-                    /* 6 - CS tolerance */
-                    /* 7 - color space associated with tolerance */
 
-                    term->vt.attrs.have_fg = 1;
+                term->vt.attrs.have_fg = 1;
-                    term->vt.attrs.fg = r << 16 | g << 8 | b;
+                term->vt.attrs.fg = r << 16 | g << 8 | b;
-                    break;
-                }
-
-                case 5: { /* Indexed - 38:2:5:<idx> */
-                    if (param->sub.idx < 3) {
-                        UNHANDLED_SGR(i);
-                        break;
-                    }
-
-                    uint8_t idx = param->sub.value[2];
-                    term->vt.attrs.have_fg = 1;
-                    term->vt.attrs.fg = term->colors.table[idx];
-                    break;
-                }
-
-                case 1: /* Transparent */
-                case 3: /* CMY */
-                case 4: /* CMYK */
-                    UNHANDLED_SGR(i);
-                    break;
-                }
             }
 
+            /* Transparent: 38:1 */
+            /* CMY:         38:3:<color-space>:c:m:y[:tolerance:tolerance-color-space] */
+            /* CMYK:        38:4:<color-space>:c:m:y:k[:tolerance:tolerance-color-space] */
+
             /* Unrecognized */
             else
                 UNHANDLED_SGR(i);
@@ -236,53 +234,51 @@ csi_sgr(struct terminal *term)
                 i += 4;
             }
 
-            /* Sub-parameter style: 48:2:... */
+            /* Indexed: 48:5:<idx> */
             else if (term->vt.params.v[i].sub.idx >= 2 &&
+                     term->vt.params.v[i].sub.value[0] == 5)
+            {
+                const struct vt_param *param = &term->vt.params.v[i];
+
+                uint8_t idx = param->sub.value[1];
+                term->vt.attrs.have_bg = 1;
+                term->vt.attrs.bg = term->colors.table[idx];
+            }
+
+            /*
+             * RGB: 48:2:<color-space>:r:g:b[:ignored:tolerance:tolerance-color-space]
+             * RGB: 48:2:r:g:b
+             *
+             * The second version is a "bastard" version - many
+             * programs "forget" the color space ID
+             * parameter... *sigh*
+             */
+            else if (term->vt.params.v[i].sub.idx >= 4 &&
                      term->vt.params.v[i].sub.value[0] == 2)
             {
                 const struct vt_param *param = &term->vt.params.v[i];
-                const int color_space_id = param->sub.value[1];
+                bool have_color_space_id = param->sub.idx >= 5;
 
-                switch (color_space_id) {
+                /* 0 - color space (ignored) */
-                case 0:   /* Implementation defined - we map it to '2' */
+                int r_idx = 2 - !have_color_space_id;
-                case 2: { /* RGB - 48:2:2:<r>:<g>:<b> */
+                int g_idx = 3 - !have_color_space_id;
-                    if (param->sub.idx < 5) {
+                int b_idx = 4 - !have_color_space_id;
-                        UNHANDLED_SGR(i);
+                /* 5 - unused */
-                        break;
+                /* 6 - CS tolerance */
-                    }
+                /* 7 - color space associated with tolerance */
 
-                    uint8_t r = param->sub.value[2];
+                uint8_t r = param->sub.value[r_idx];
-                    uint8_t g = param->sub.value[3];
+                uint8_t g = param->sub.value[g_idx];
-                    uint8_t b = param->sub.value[4];
+                uint8_t b = param->sub.value[b_idx];
-                    /* 5 - unused */
-                    /* 6 - CS tolerance */
-                    /* 7 - color space associated with tolerance */
 
-                    term->vt.attrs.have_bg = 1;
+                term->vt.attrs.have_bg = 1;
-                    term->vt.attrs.bg = r << 16 | g << 8 | b;
+                term->vt.attrs.bg = r << 16 | g << 8 | b;
-                    break;
-                }
-
-                case 5: { /* Indexed - 48:2:5:<idx> */
-                    if (param->sub.idx < 3) {
-                        UNHANDLED_SGR(i);
-                        break;
-                    }
-
-                    uint8_t idx = param->sub.value[2];
-                    term->vt.attrs.have_bg = 1;
-                    term->vt.attrs.bg = term->colors.table[idx];
-                    break;
-                }
-
-                case 1: /* Transparent */
-                case 3: /* CMY */
-                case 4: /* CMYK */
-                    UNHANDLED_SGR(i);
-                    break;
-                }
             }
 
+            /* Transparent: 48:1 */
+            /* CMY:         48:3:<color-space>:c:m:y[:tolerance:tolerance-color-space] */
+            /* CMYK:        48:4:<color-space>:c:m:y:k[:tolerance:tolerance-color-space] */
+
             else
                 UNHANDLED_SGR(i);
 

foot.info

@@ -1,15 +1,15 @@
 foot|foot terminal emulator,
     use=foot+base,
     colors#256,
-    setab=\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48\:2\:5\:%p1%d%;m,
+    setab=\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48\:5\:%p1%d%;m,
-    setaf=\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38\:2\:5\:%p1%d%;m,
+    setaf=\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38\:5\:%p1%d%;m,
 
 foot-direct|foot with direct color indexing,
     use=foot+base,
     colors#16777216,
     RGB,
-    setab=\E[%?%p1%{8}%<%t4%p1%d%e48\:2\:2\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m,
+    setab=\E[%?%p1%{8}%<%t4%p1%d%e48\:2\:\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m,
-    setaf=\E[%?%p1%{8}%<%t3%p1%d%e38\:2\:2\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m,
+    setaf=\E[%?%p1%{8}%<%t3%p1%d%e38\:2\:\:%p1%{65536}%/%d\:%p1%{256}%/%{255}%&%d\:%p1%{255}%&%d%;m,
 
 foot+base|foot base fragment,
     am,