XResources reload patch mistake

[og900aero]

I read the following description from patches.def.h

/* This patch adds the ability to reload the Xresources config when a SIGUSR1 signal is received
 * e.g.: killall -USR1 st
 * Depends on the XRESOURCES_PATCH.
 */
#define XRESOURCES_RELOAD_PATCH 0

But when I see from web:

https://st.suckless.org/patches/xresources-with-reload-signal/

It is better, because not depends on Xrsources patch, and it is not necessary to define separately what color scheme to start with (config.def.h)
Maybe this patch could also be included? ( I used st-xresources-signal-reloading-20220407-ef05519.diff)

[bakkeby]

This is essentially the same as #116.

The only thing that xresources-with-reload-signal patch has going for it is that the resources that are loaded are hardcoded and you can't configure that in your config.h. As such I would ask what the benefit of that is?

It is better, because not depends on Xrsources patch

The XRESOURCES_RELOAD_PATCH is an optional addon for the xresources patch.

As such the dependency is that you also enable XRESOURCES_PATCH as well, that's really all there is to it.

[og900aero]

Yes, it is the same for #116.

But I don't need xresources_patch ls signal reload partch. In addition, its code is more complicated, and you have to put color codes in config.def.h that will start st. This is completely pointless, as you should get it from xresources. That's why the patch I showed is better, because it's one patch in total, and you don't need to write any color schemes in config.h, but actually take it from the Xresources file.

[bakkeby]

But I don't need xresources_patch ls signal reload partch.

If you don't need signal reloading then you don't need this patch.

and you have to put color codes in config.def.h

I don't get it. Please explain to me like I am 5.

[og900aero]

Then I worded it wrong. For me and the author of the other post, signal reloading is needed, but in a way that does not require the Xresources patch, because it solves the use of the color scheme with hardcode, which violates the spirit of st, which is to keep the code simple. Thanks to the patch I linked, the color scheme should not be implemented in the st config file, but in the .Xresources file (which is used by several programs anyway), so the st code will be smaller and cleaner, and you won't need two patches, just 1.

[bakkeby]

From my perspective I would need a good reason to add a patch that offers the same functionality as another patch that has already been integrated.

If you had read and understood both patches then you may have realised that neither patch absolves you from defining a default colour scheme in your config as this comes from a bare unpatched st:

https://git.suckless.org/st/file/config.def.h.html#l96

Neither patch removes this.

Both patches read Xresources so you are not getting around that. The point of using either of the patches is to load colours from Xresources.

The xresources patch allows the end user to control what values are read from Xresources via the resources array, whereas the xresources-with-reload-signal hardcodes this.

One of the patches is implemented cleanly, the other is not.

The XRESOURCES_RELOAD_PATCH toggle only adds the function to reload Xresources and setting up the signal handling, so the only real difference comes down to the Xresources implementation.

Overall my interpretation of everything you have said so far is that the xresources-with-reload-signal patch is superior to the xresources patch because then you can set this in patches.h:

#define XRESOURCES_WITH_RELOAD_SIGNAL_PATCH 1

rather than

#define XRESOURCES_PATCH 1
#define XRESOURCES_RELOAD_PATCH 1

And having two toggles is a bit too much for you.

[og900aero]

The XRESORCES patch you implemented doesn't read the color code from the .Xresources file for me, but instead takes the code from config.h. If I write it there, it works, if I don't write it, it doesn't extract it from the .XResources file. I tried it before posting the ticket here. And obviously it is, if someone else has already indicated this to you.

https://st.suckless.org/patches/xresources-with-reload-signal/ st-xresources-signal-reloading-20220407-ef05519.diff
patch, on the other hand, actually takes the color scheme from the .XResources file, and the reload function works at the same time.

However, I'm starting to feel that you're teasing and making excuses rather than trying to understand the difference, despite the fact that someone else besides me has pointed this out.
I think it's easier to write that you don't want to implement this in your solution, and that's it, you can close the ticket.

[bakkeby]

This is a request to include a specific patch. All I am asking for here is a single good reason to include this patch.

I'm starting to feel that you're teasing and making excuses

I can assure you that I am not teasing. I do not have time for that and this is quite frankly a major waste of time for me.

The XRESORCES patch you implemented doesn't read the color code from the .Xresources file for me, but instead takes the code from config.h. If I write it there, it works, if I don't write it, it doesn't extract it from the .XResources file.

Have you considered why that may be?

An XResources patch that doesn't read any resources on startup doesn't sound like it would be doing its job. So far I haven't had any reports of this not working, nor can I replicate.

Do you have any clear instructions on how to reproduce this issue?

Just to clarify neither patch reads from the .Xresources file; they read from the RESOURCE_MANAGER property of the root window. This property gets set when you run xrdb on the resource file.

I don't really have a problem including this patch, it is trivial to add and doesn't conflict with anything. It just seems a bit redundant that's all.

[og900aero]

I'm closing the ticket. I prefer to do it by hand so that there is no accidental "waste of time" in any development.