Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support x:<name> syntax to fetch color definition from .Xresources #2044

Merged

Conversation

jkorinth
Copy link
Contributor

Adds support to retrieve simple color definitions from ~/.Xresources with a new prefix in the color definitions in theme overrides, e.g.,

[theme.overrides]
idle-bg = "x:background"

would look for a line like

*background: #feedda

in ~/.Xresources.

@MaxVerevkin
Copy link
Collaborator

MaxVerevkin commented Apr 14, 2024

This looks simple enough, so why not.

This feature should be documented here.

To make testing easier, I would prefer to have a dedicated function called extract_xresources_colors(content: &str) -> HashMap<String, String>. Then in tests, you will not have to mock read_xresources function, but instead just assert! the contents of the returned hashmap. Edit: I see, you need mocking to test the FromStr impl.

@jkorinth jkorinth force-pushed the feature/support_xresources_colors branch from be88b17 to 70e19e8 Compare April 14, 2024 18:32
@jkorinth
Copy link
Contributor Author

Updated with the suggestions. There's no documentation for the 'hsv:` prefix, but I think that should go into a separate PR.

@jkorinth jkorinth force-pushed the feature/support_xresources_colors branch from 129b8b6 to e084ef4 Compare April 19, 2024 21:19
@jkorinth
Copy link
Contributor Author

Btw: errors deserves an overhaul, too, I think. Check out anyhow and thiserror, they eliminate most of the boiler plate.

@MaxVerevkin
Copy link
Collaborator

Btw: errors deserves an overhaul, too, I think. Check out anyhow and thiserror, they eliminate most of the boiler plate.

I know about these crates and we use thiserror already, in same cases. Creating an error enum for each block does not seem practical. We are not using anyhow because in the past Error struct used to contain a block: String field. It no longer does (we now have a separate BlockError for this, which uses thiserror) and I think we can switch to anyhow now.

@jkorinth jkorinth force-pushed the feature/support_xresources_colors branch from fb4244c to fa06d0d Compare April 21, 2024 14:07
src/themes/xresources.rs Outdated Show resolved Hide resolved
src/themes/xresources.rs Outdated Show resolved Hide resolved
src/themes/xresources.rs Outdated Show resolved Hide resolved
@MaxVerevkin
Copy link
Collaborator

LGTM. Just needs the CI to be passing.

src/themes/color.rs Outdated Show resolved Hide resolved
src/themes/color.rs Outdated Show resolved Hide resolved
@jkorinth
Copy link
Contributor Author

LGTM. Just needs the CI to be passing.

How do you tell cspell to ignore words?

@MaxVerevkin
Copy link
Collaborator

How do you tell cspell to ignore words?

Edit cspell.yaml

@jkorinth
Copy link
Contributor Author

How do you tell cspell to ignore words?

Edit cspell.yaml

Wait - cspell spellchecks not only the docs, but also the code itself?! And not just strings inside, but the full code? 😂

@jkorinth jkorinth force-pushed the feature/support_xresources_colors branch from d05b224 to 2b34f40 Compare April 23, 2024 19:30
@MaxVerevkin
Copy link
Collaborator

How do you tell cspell to ignore words?

Edit cspell.yaml

Wait - cspell spellchecks not only the docs, but also the code itself?! And not just strings inside, but the full code? 😂

Yea, it is sometimes annoying, but now we have less typos in variable names, etc.

@MaxVerevkin
Copy link
Collaborator

@jkorinth I've pushed a commit simplifying some stuff, can you review and test it?

@jkorinth
Copy link
Contributor Author

jkorinth commented Sep 8, 2024

Sorry, forgot about this. LGTM!

@bim9262 bim9262 force-pushed the feature/support_xresources_colors branch from a037dc1 to 369147f Compare January 5, 2025 02:06
@bim9262
Copy link
Collaborator

bim9262 commented Jan 5, 2025

Fixed cspell merge error and use std::sync::LazyLock instead of once_cell::sync::Lazy

@bim9262 bim9262 merged commit 3785330 into greshake:master Jan 5, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants