Cannot compile without the sixel patch

[veltza]

This is what happens when you compile st-flexipatch without the sixel patch:

> make
cp config.def.h config.h
cp patches.def.h patches.h
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`   -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c st.c
In file included from st.c:21:
st.c: In function ‘strhandle’:
st.c:2465:60: error: ‘osc_table’ undeclared (first use in this function)
 2465 |                         if ((j = par - 10) < 0 || j >= LEN(osc_table))
      |                                                            ^~~~~~~~~
st.h:17:41: note: in definition of macro ‘LEN’
   17 | #define LEN(a)                  (sizeof(a) / sizeof(a)[0])
      |                                         ^
st.c:2465:60: note: each undeclared identifier is reported only once for each function it appears in
 2465 |                         if ((j = par - 10) < 0 || j >= LEN(osc_table))
      |                                                            ^~~~~~~~~
st.h:17:41: note: in definition of macro ‘LEN’
   17 | #define LEN(a)                  (sizeof(a) / sizeof(a)[0])
      |                                         ^
make: *** [Makefile:19: st.o] Error 1

The reason is that osc_table is inside a sixel block when it shouldn't be inside any block.

---

I also have another issue related to this commit. I never applied that upstream patch to st-sx because it broke my shell prompts (bash and zsh) when I pasted emojis from the clipboard into the prompt.

Here is how to reproduce it:

1. Compile st-flexipatch without patches and start it.
2. Type echo
3. Paste an emoji from the clipboard into the prompt five times. (You should see how strangely the prompt behaves)
4. Press enter to see that the command works correctly even though the prompt is messed up.

Can you confirm this behavior or is there something wrong with my setup?

[bakkeby]

Yes could reproduce that issue, reverted the commit, thanks.

[veltza]

Oh boy, what the hell were they thinking when they approved that breaking patch?

[bakkeby]

It was getting pretty late yesterday, misplacing the osc_table was rather sloppy :)

It is pretty minor but I wanted to mention that while going through the many changes you have made in your build I spotted some curly braces missing here: https://github.com/veltza/st-sx/blob/efe8286760dfb7171f58d69b43e39c4ab56f595c/st.c#L2915-L2919

Introduced via this commit: https://github.com/veltza/st-sx/commit/98ced789e4e5a6c5473c1fb42434e92fc0c0ea3b

[veltza]

Thanks. Apparently I'm good at finding other people's bugs, but I can't find my own. lol

The reason I wanted to support both OSC sequence terminators (BEL and ST) was to maintain backwards compatibility and support modern applications like vv that expect the OSC color response to end with ST. So when either terminator is used in the request, the response uses the same one. Some terminals like xterm and XFCE terminal also work this way, while others like WezTerm always use ST in the response. The vanilla st uses BEL.

[veltza]

I just noticed that the ligature patch won't compile without the vertcenter patch:

> make
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`  `pkg-config --cflags harfbuzz` -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c st.c
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`  `pkg-config --cflags harfbuzz` -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c x.c
x.c: In function ‘xmakeglyphfontspecs’:
x.c:1634:50: error: ‘TermWindow’ has no member named ‘cyo’
 1634 |         xp = winx, yp = winy + font->ascent + win.cyo;
      |                                                  ^
make: *** [Makefile:19: x.o] Error 1

And when I enabled the vertcenter as well, the terminal crashed when I started it:

> make
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`  `pkg-config --cflags harfbuzz` -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c st.c
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`  `pkg-config --cflags harfbuzz` -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c x.c
c99 -I/usr/X11R6/include  `pkg-config --cflags fontconfig`  `pkg-config --cflags freetype2`  `pkg-config --cflags harfbuzz` -DVERSION=\"0.9\" -D_XOPEN_SOURCE=600  -O1 -c hb.c
c99 -o st st.o x.o hb.o -L/usr/X11R6/lib -lm -lrt -lX11 -lutil -lXft    `pkg-config --libs fontconfig`  `pkg-config --libs freetype2`  `pkg-config --libs harfbuzz` 
> ./st
zsh: segmentation fault (core dumped)  ./st

I'm not quite sure what's causing this, but I can see that this conditional block is not correct:

#if WIDE_GLYPHS_PATCH
xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, int y, int dmode, int charlen)
#else
xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, int y)
#endif // WIDE_GLYPHS_PATCH

It is the ligature patch that requires charlen parameter, not the wide glyph patch, as you can see here:

 void
-xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, int y)
+xdrawglyphfontspecs(const XftGlyphFontSpec *specs, Glyph base, int len, int x, int y, int charlen)
 {
-   int charlen = len * ((base.mode & ATTR_WIDE) ? 2 : 1);

But there might be something else too.

[bakkeby]

You are right, I shouldn't be doing these changes so late in the evening.

I got confused because both the ligatures and the wide glyphs patches change the signature of xdrawglyphfontspecs. Commited some fixes for that. I was able to spawn terminals with combinations of ligatures, wide glyphs and vertcenter so let me know if you still get the segfault when enabling ligatures.

[veltza]

I found the crashing bug. The ligatures block is inside the xresources block, so ligatures only work when the xresources patch is also enabled:

    #if XRESOURCES_PATCH
    if (!(xw.dpy = XOpenDisplay(NULL)))
        die("Can't open display\n");

    #if LIGATURES_PATCH
    hbcreatebuffer();
    #endif // LIGATURES_PATCH

    config_init(xw.dpy);
    #endif // XRESOURCES_PATCH

[bakkeby]

Thanks for catching that. xresources is obviously one I had enabled.