profanity-im / profanity

Ncurses based XMPP client
https://profanity-im.github.io/
Other
1.33k stars 187 forks source link

Allow theme colors to be specified as integers #1903

Open jubalh opened 1 year ago

jubalh commented 1 year ago

This PR got opened by @jubalh for toastal who sent his patch via mailinglist and in the profanity MUC.

jubalh commented 1 year ago

Can't set myself as reviewer by a PR opened by myself.. Great. Anyways .. I'll have to review this later. Not just opened PR so the patch doesn't get lost.

jubalh commented 1 year ago

ul could be renamed to something like col_value/col_number

done!

I guess we probably should document this possibility somewhere/add an example?

H3rnand3zzz commented 1 year ago

I guess we probably should document this possibility somewhere/add an example?

that would be great finishing touch

jubalh commented 11 months ago

toastal mentioned he later found a problem with his patch regarding background colors. I asked several times what problem exactly and how to reproduce but so far didn't get an answer.

I'm considering to close this PR until a new patch that is known to work correctly, or way to reproduce the issue is given.

toastal commented 11 months ago

I had mentioned the issue. My patch only worked on background colors (titlebar, statusbar, roster.header), but not the other colors. I have never written C so I don’t know how to go about this any more than the time I spent on this.

The code has an explicit block warning about problematic color names & it seems an odd limitation to make users supply strings that need to reverse lookup the matching uint8 color when the alternative of just allowing uint8 values to be put in the config instead of the strings should be a bit quicker, but also allow users to get to the colors with duplicate names.

toastal commented 11 months ago

If anyone would like to take it over the line themselves or pairing I would assist (especially testing), but I don’t have the skill to finish this--I don’t even know how to properly debug it either… was just shooting at the wall & seeing what stuck.

H3rnand3zzz commented 11 months ago

If anyone would like to take it over the line themselves or pairing I would assist (especially testing), but I don’t have the skill to finish this--I don’t even know how to properly debug it either… was just shooting at the wall & seeing what stuck.

Can you just provide some steps to reproduce a problem? E.g. specific setting in theme. Once it's reproducible, it will be easy to fix.

toastal commented 11 months ago

@H3rnand3zzz

Here were some colors I was trying

[colours]
bkgnd=default
titlebar=0x36
titlebar.text=0x07
titlebar.brackets=0xB7
titlebar.unencrypted=0xC5
titlebar.encrypted=0x94
titlebar.untrusted=0xD6
titlebar.trusted=0x94
titlebar.online=0x07
titlebar.offline=0x87
titlebar.away=0x87
titlebar.chat=0x07
titlebar.dnd=0x63
titlebar.xa=0x07
statusbar=0xEC
statusbar.text=0xF7
roster.header=0xA2
roster.chat=0x63
roster.online=0x8D

[ui]
time.lastactivity=%y-%m-%d %H:%M:%S
time.vcard=%y-%m-%d %H:%M:%S
time.muc=%m-%d %H:%M:%S
toastal commented 11 months ago

A program with desired behavior, meli

VALID COLOR VALUES

Color values are of type String with the following valid contents: […] 0-255 byte for 256 colors. xterm(1) name but with some modifications (for a full table see COLOR NAMES addendum) (Case-sensitive)

https://meli-email.org/documentation.html#meli-themes.5_VALID_COLOR_VALUES

I’m going to assume they chose to make modifications since there are name overlaps in the xterm names (not that the descriptions are great)