dcat / st-xresources

simple terminal with Xresource support
MIT License
43 stars 4 forks source link

[QUESTION] xresource not defined take any value #3

Open flohw opened 5 years ago

flohw commented 5 years ago

Hello!

I try to use your patch to use my Xresources file to theme my st. I applied the patch properly and modified my config.h accordingly.

In first place It looks like it doesn't work properly. After some research and tries it looks like the background, foreground and cursorColor are not linked properly with the original colorname array.

The colorname indexes for background, foreground and cursorColor are 0, 7 and 256 while your patch refers to 256, 257 and 258.

Is your patch provided in the st patches related to your specific needs or should it stuck to the st default configuration ?

Whouldn't be better to provide a patch which match st default configuration?

Thanks

nick-chandoke commented 5 years ago

Thank you! I just discovered st, and applied the patch, wondering why background color wasn't applying. Remapping the indicies as you've indicated works perfectly.

flohw commented 5 years ago

@nick-chandoke hi! maybe the variable defaultbg (line 120 on the original config.def.h is not set to the correct index. It should reflext the color index in the colorname array defined above in the same file. Index starts at zero. If you want the gray90 as the background color. The defaultbg value should be 7. If you want the #cccccc color as the background color, defaultbg should be set to 256.

nick-chandoke commented 5 years ago

@flohw I suppose that I could change defaultbg rather than the indices in the patch. I hadn't modified config.h since cloning st, though. I expected that I could clone st, apply the Xresources patch, compile, then use Xresource code generated by http://terminal.sexy. It seems odd that there's a need to modify config.h first. I mean how would anyone who hasn't used st before know to do that?

flohw commented 5 years ago

All depends on how you manage your st sources and the patch. I decided to make the patch compatible with the defaut st sources so I updated the patch rather than change the defaultbg value.

This is explained in st documentation. This is your config.h file and you have to update it manually when you apply a patch that updates the config.def.h file as the make command didn't update your config.h to prevent override with your custom configuration. I think that st is not made for beginners or people that didn't know C or development basics.

The only thing I would suggest it that any one who want to make a patch for st should make it on top of a tag name (providing the patch also for the master branch would be a great addition) without any changes to the default config.def.h existing values such as the indexes referenced in my issue in order to make the patch works properly with a config.h built from the default config.def.h file from st. (I hope this paragraph is clear, it's not always easy to translate my french though to english words :smile: )

Checkout my sources repository if you want, the readme should be okay. :slightly_smiling_face:

Eksko commented 5 years ago

@flohw Hello and thank you for posting the correction for the colorname indexes. :) At first I thought I did something wrong while patching st as I am relatively new to this kind of installations, but seems like the patch itself was wrong.