Add Xresources-based theme

darrenburns / posting

The modern API client that lives in your terminal.

Apache License 2.0

4.48k stars 64 forks source link

Add Xresources-based theme #35

Closed SqrtMinusOne closed 1 month ago

SqrtMinusOne commented 2 months ago

I use Xresources to configure themes. This PR adds themes called xresources-light and xresources-dark, provided that xrdb is found and xrdb -query returns the necessary colors.

This is a bit suboptimal for light theme (which I use), because light themes don't need as much shading as C-o and C-p currently do. But solarized-light also has this issue.

darrenburns commented 2 months ago

Thanks for the contribution! - I've done a short review, let me know what you think or if you need help.

SqrtMinusOne commented 1 month ago

Done. I've:

add an option called use_xresources, as you suggested;
changed the method to fail with more verbose errors in case something goes wrong;
moved the invocation of the method from on_mount to the constructor. I'm not sure if there's a more suitable place for it, but it works.

SqrtMinusOne commented 1 month ago

Oops, my Emacs has auto-formatted the table in the README. I hope you don't mind.

darrenburns commented 1 month ago

Thanks! I'll try to review this this week.

darrenburns commented 1 month ago

Thank you - I've made a few minor changes. Moved stuff into a new module to keep it separate from the App, an simplified the code a bit.

Would you mind giving it a quick test? If it looks good to you I will merge.

SqrtMinusOne commented 1 month ago

Thanks.

But the current version fails with:

  File "/home/pavel/00-Scratch/posting/src/posting/xresources.py", line 36, in load_xresources_themes
    missing_colors = set(XRDB_MAPPING.values()) - supplied_colors.keys()
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: unhashable type: 'list'

The point of list(itertools.chain(*self.XRDB_MAPPING.values())) was to flatten that list. I'll make a new PR with a fix.