Hi, Some comments below:
On Fri, Dec 10, 2021 at 05:53:40PM -0500, Raheman Vaiya wrote: > The attached patch adds full support for the OSC 10/11/12/4 control > sequences. > These are used by programs like vim and theme.sh to query and set terminal > colours and are supported by most major terminals (e.g alacrittty, kitty, > libvte). I submitted a patch to the wiki some time ago, but it didn't xterm supports it too. > include > support for query sequences. I am posting it here because I feel it would > be a > good candidate for inclusion into st itself. > > You can test it using https://github.com/lemnos/theme.sh by running > > theme.sh zenburn > theme.sh -p > > Don't hesitate to let me know if I have made any grievous errors :P. > > Regards, > Raheman > diff --git a/config.def.h b/config.def.h > index 6f05dce..3932e83 100644 > --- a/config.def.h > +++ b/config.def.h > @@ -127,9 +127,9 @@ static const char *colorname[] = { > * Default colors (colorname index) > * foreground, background, cursor, reverse cursor > */ > -unsigned int defaultfg = 7; > -unsigned int defaultbg = 0; > -static unsigned int defaultcs = 256; > +unsigned int defaultfg = 255; > +unsigned int defaultbg = 254; > +unsigned int defaultcs = 256; > static unsigned int defaultrcs = 257; The values in the config are changed here. Is it intended? If so, why? > > /* > diff --git a/st.c b/st.c > index a9338e1..a469512 100644 > --- a/st.c > +++ b/st.c > @@ -1842,6 +1842,32 @@ csireset(void) > memset(&csiescseq, 0, sizeof(csiescseq)); > } > > +void > +osc4_color_response(int num) > +{ > + int n; > + char buf[32]; > + unsigned char r,g,b; > + > + xgetcolor(num, &r,&g,&b); > + n = sprintf(buf, "\033]4;%d;rgb:%02x%02x/%02x%02x/%02x%02x\007", num, > r, r, g, g, b, b); > + I prefer snprintf here, just in case. The arguments for sprintf are too long and should be wrapped. > + ttywrite(buf, n, 1); > +} > + > +void > +osc_color_response(int index, int num) > +{ > + int n; > + char buf[32]; > + unsigned char r,g,b; > + > + xgetcolor(index, &r,&g,&b); > + n = sprintf(buf, "\033]%d;rgb:%02x%02x/%02x%02x/%02x%02x\007", num, r, > r, g, g, b, b); > + > + ttywrite(buf, n, 1); > +} > + > void > strhandle(void) > { > @@ -1880,6 +1906,45 @@ strhandle(void) > } > } > 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 %d\n", p); The argument p is not int. The compiler (clang) also gives a warning about it. > + else > + redraw(); > + break; > + case 12: > + if (narg < 2) > + break; > + > + p = strescseq.args[1]; > + > + if(!strcmp(p, "?")) > + osc_color_response(defaultcs, 12); > + else if (xsetcolorname(defaultcs, p)) > + fprintf(stderr, "erresc: invalid cursor color > %d\n", p); > + else > + redraw(); > + break; > + case 10: Case 10 should be ordered before case 11. > + 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 %d\n", p); > + else > + redraw(); > + break; > case 4: /* color set */ > if (narg < 3) > break; > @@ -1887,7 +1952,10 @@ strhandle(void) > /* FALLTHROUGH */ > case 104: /* color reset, here p = NULL */ > j = (narg > 1) ? atoi(strescseq.args[1]) : -1; The variable j should be checked and also the xgetcolor and/or functions using it should have range checks. It seems dc.col can be out-of-bounds with some input. I also noticed a crash here while testing. > - if (xsetcolorname(j, p)) { > + > + if(!strcmp(p, "?")) > + osc4_color_response(j); > + else if (xsetcolorname(j, p)) { > if (par == 104 && narg <= 1) > return; /* color reset without > parameter */ > fprintf(stderr, "erresc: invalid color j=%d, > p=%s\n", > diff --git a/st.h b/st.h > index fa2eddf..47d76ca 100644 > --- a/st.h > +++ b/st.h > @@ -107,6 +107,7 @@ char *getsel(void); > > size_t utf8encode(Rune, char *); > > +void xgetcolor(int x, unsigned char *r, unsigned char *g, unsigned char *b); xgetcolor should be below xstrdup and as a separate line. > void *xmalloc(size_t); > void *xrealloc(void *, size_t); > char *xstrdup(const char *); > @@ -123,3 +124,4 @@ extern char *termname; > extern unsigned int tabspaces; > extern unsigned int defaultfg; > extern unsigned int defaultbg; > +extern unsigned int defaultcs; > diff --git a/x.c b/x.c > index 89786b8..3761fd0 100644 > --- a/x.c > +++ b/x.c > @@ -799,6 +799,14 @@ xloadcols(void) > loaded = 1; > } > > +void > +xgetcolor(int x, unsigned char *r, unsigned char *g, unsigned char *b) > +{ > + *r = dc.col[x].color.red >> 8; > + *g = dc.col[x].color.green >> 8; > + *b = dc.col[x].color.blue >> 8; > +} > + > int > xsetcolorname(int x, const char *name) > { This patch needs a bit more care and testing to be able to accept it. Thanks for your efforts, -- Kind regards, Hiltjo
