OpenBSD pledge & build issues

[twoheadedsun]

There are some issues if you are running OpenBSD:

If you are using SIXEL_PATCH, in config.mk the OpenBSD section should also include the Sixel libraries as well, otherwise it won't build.

The patch XRESOURCES_RELOAD_PATCH needs "unix" in pledge, otherwise st crashes when you give it the reload signal. Also the command for refreshing should not be "killall -USR1 st" but "pkill -USR1 st" if you are on OpenBSD.

[bakkeby]

I wonder if overwriting LIBS in the OpenBSD section in config.mk is necessary. The only difference that I can see is that the first (default) includes -lrt which is the realtime extensions library.

https://git.suckless.org/st/file/config.mk.html

For me it compiles fine without it. Maybe it is part of the standard libraries or something.

Perhaps it was added for the clock_gettime calls.

https://refspecs.linuxbase.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/app-librt.html

The only reference to "killall" is as an example in patches.h. I suppose that it would be possible to also include pkill in the example. Maybe the program works differently on OpenBSD, but on Linux it seems that it needs an additional -x argument "pkill -USR1 -x st" as otherwise it try to kill every process that has "st" in its name (e.g. slstatus).

As for the pledge I think that we can try something like this:

    default:
#ifdef __OpenBSD__
        if (pledge(
            "stdio rpath tty proc"
            #if RIGHTCLICKTOPLUMB_PATCH || OPENCOPIED_PATCH
            " ps exec"
            #endif // RIGHTCLICKTOPLUMB_PATCH | OPENCOPIED_PATCH
            #if XRESOURCES_PATCH && XRESOURCES_RELOAD_PATCH || BACKGROUND_IMAGE_PATCH && BACKGROUND_IMAGE_RELOAD_PATCH
            " unix"
            #endif // XRESOURCES_RELOAD_PATCH | BACKGROUND_IMAGE_RELOAD_PATCH
            , NULL
        ) == -1)
            die("pledge\n");
#endif

[twoheadedsun]

Regarding config.mk:

if I leave this section intact:

# OpenBSD:
CPPFLAGS = -DVERSION=\"$(VERSION)\" -D_XOPEN_SOURCE=600 -D_BSD_SOURCE
LIBS = -L$(X11LIB) -lm -lX11 -lutil -lXft \
       `pkg-config --libs fontconfig` \
       `pkg-config --libs freetype2`
MANPREFIX = ${PREFIX}/man

and compile on OpenBSD, I get errors like this:
ld: error: undefined symbol: imlib_create_image_using_data

referenced by x.c
x.o:(xfinishdraw)
ld: error: undefined symbol: imlib_context_set_image
referenced by x.c
x.o:(xfinishdraw)
cc: error: linker command failed with exit code 1 (use -v to see invocation)

but if I change it to this:

LIBS = -L$(X11LIB) -lm -lX11 -lutil -lXft ${SIXEL_LIBS} ${XRENDER} ${XCURSOR}\
       `pkg-config --libs fontconfig` \
       `pkg-config --libs freetype2`

it compiles fine.

I think your pledge change is fine. One thing I want to mention is that i had to do a similar thing for dwm xresources reloading - I had to add "unix" to its pledge. I am not using your flexipatch for dwm and patched my version manually, but I had to do this change otherwise when i used the "xrdb" patch, which allows for live reloading with win+f5, X crashed . Maybe this is something you might consider looking into.

Another thing that I just noticed - when i do the "pkill -USR1 -x st" command on a st terminal with a changed font size - the font gets reset to its default value instead of keeping the set value.

Also, if you don't mind answering here, could you elaborate on your comment in the TODO file "add diacritics support to xdraws()"
I ask because I wonder if you mean combined unicode, because I would love that feature. I read Greek polytonic letters often on my terminal and st is not capable of displaying letters like ᾱ́, which is alpha with both an acute and a macron diacritic. Basically any Greek letter with a diacritic + macron isn't properly displayed. This site can be used to generate these letters if you want to test yourself: https://philolog.us/hoplitekeyboard.html

[bakkeby]

Regarding config.mk

I was more thinking that LIBS is defined twice in config.mk, the second time to be uncommented when using OpenBSD.

I am questioning whether there is any legitimate need to define LIBS twice considering that they are practically identical (less the inclusion of -lrt).

One thing I want to mention is that i had to do a similar thing for dwm xresources reloading - I had to add "unix" to its pledge. I am not using your flexipatch for dwm and patched my version manually, but I had to do this change otherwise when i used the "xrdb" patch, which allows for live reloading with win+f5, X crashed. Maybe this is something you might consider looking into.

I think it is probably just that patch creators do not tend to test things under OpenBSD so things like pledges can be missed. It is a good opportunity for OpenBSD users to contribute to patches when issues are found.

I'll look into this for dwm-flexipatch.

Another thing that I just noticed - when i do the "pkill -USR1 -x st" command on a st terminal with a changed font size - the font gets reset to its default value instead of keeping the set value.

I think that is to be expected I think considering that the Xresources patch for st allows for the font to be changed via resources, hence it is going to reload fonts as well.

If you don't like this then you can try commenting out the lines that reloads fonts:

st-flexipatch/patch/xresources.c

Lines 66 to 71 in f8e451e

	/* nearly like zoomabs() */
	xunloadfonts();
	xloadfonts(font, 0); /* font <- config_init() */
	#if FONT2_PATCH
	xloadsparefonts();
	#endif // FONT2_PATCH

(disclaimer: I have not tested this to confirm that it works)

Also, if you don't mind answering here, could you elaborate on your comment in the TODO file "add diacritics support to xdraws()"

The TODO file is merely inherited from the upstream project and the diacritics comment was added back in 2013 ref.

https://git.suckless.org/st/commit/502911e55442a89602bd5681763f68250bff330b.html

Diacritics seem to work in some terminals such as Kitty, gnome-terminal and iTerm, but not in xterm and Alacritty as far as I could find.

I am not entirely sure what would be involved to support this in st.

[twoheadedsun]

Regarding config.mk: I understand what you mean now. I'm not sure why it was written like that in the original file. Maybe there is a reason, but I am not competent enough to answer.

I actually emailed one of the xrdb patch developers regarding the pledge bug. I just read the email again and I see that you had to add not just "unix" but also "inet".

I tested commenting out those lines, and it works just like I want it. Thank you!

Sorry about the TODO question. I didn't think of checking if it was part of the upstream project. Thanks for suggesting other terminals with that support. Hopefully st gets diacritic support one day as well.

[veltza]

Hopefully st gets diacritic support one day as well.

There is a pending upstream patch for that. I recommend that you try it out and send feedback there.

[twoheadedsun]

Thank you very much for pointing me to this patch. I just applied it and it seems to do exactly what I want. I will stay with to continue testing it out.
The reflow patch will probably need to be adapted if this gets merged into the main branch since a bunch of boxes get drawn randomly if I leave it on. But I just disabled it for now. I can live without reflow. I'm not clever enough to make it work with diacritic support.
So far the only bugs I see is that if you paste many of these double-diacritics next to each other and start pressing backspace, it starts deleting the shell's prompt. But that doesn't seem to be an issue caused by this patch, since this happens in the original version of st as well. The only difference is that now you can see the text displayed properly, which is this patch's main job.
Also if I go overboard and add a couple of diacritics too many, it fails to load all of them, like this one ᾱ̓́ͅ. But honestly I am happy that ᾱ́ renders properly, since the other case is just so rare that you would never see it. I will write to the author of the patch what I found so far.
Thank you again for bringing his work to my attention.

[bakkeby]

The email did say that it only supports a single diacritic;

diacritics will now combine, but not stack

It looks like kitty do end up combining them:

The changes in that patch are fairly involved and will affect things like ligatures as well. From a technical perspective I am not sure if making a single diacritic part of the Glyph struct is the right way to go.

[twoheadedsun]

The changes in that patch are fairly involved and will affect things like ligatures as well.

Yes, the wide glyphs patch as well in some places.

From a technical perspective I am not sure if making a single diacritic part of the Glyph struct is the right way to go.

Maybe you are right. But I'm glad that at least some work is happening on that front.

One interesting thing that I just saw, now that you mentioned kitty, is that the shell prompt bug happens there as well. Same with alacritty. Seems to be a widely ignored bug.