-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add support for rgb and indexed colors #193
Conversation
658c2c6
to
8c292c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
(Renaming theme::theme to theme::base is a very welcome change)
Your crate sorta makes |
Hmm I haven't really looked at |
Actually now that I took a look it seems that MacchinaColor was a wrapper enum to use Color with clap. Now that it's not required, we can probably remove it. |
It implements Deserializer and Serializer, that's about it. And your crate implements that as well, so how should we proceed? |
There's a |
I've been playing around with |
Before I forget, this should probably be merged into |
Removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we ready to merge to the v6
branch?
I'd rather bundle up our new features into one major release this time.
It's core logic is done. Ill just need to clean up the code a bit, add better docs, and add a few more tests. I'll probably publish it on saturday after all is done. |
Take your time, v6 is probably a long way from being ready. |
Is the name of the crate alright ?. I was thinking to name it similar to Any suggestions ? |
If I were you, I'd name it the way I name most of my crates, in Esperanto. So "koloro" in this case. |
I mean if I made a standalone crate I might consider it. But this is just a helper crate of sorts, so I think it might be better to name it descriptive. |
Then (as it does more than just hexadecimal colors) |
Ready to merge into |
Yes. Let me know if something needs to be changed. |
As far as I can see, everything looks great :) |
Also published to crates.io
Rebased to v6 branch. |
I think we need to show the parsing errors to the users. I can't figure out what is wrong with |
We really do, in the meantime, can you show me the file? |
# for a complete reference, visit
# https://github.com/Macchina-CLI/macchina/wiki/Theme-Documentation
spacing = 2
padding = 0
hide_ascii = false
hide_bar_delimiters = true
prefer_small_ascii = false
separator = ">"
key_color = "#ff6135"
separator_color = "#0ac365"
palette = "Light"
[bar]
glyph = "ߋ"
symbol_open = '['
symbol_close = ']'
visible = true
[box]
title = "aurc"
visible = true
[box.inner_margin]
x = 1
y = 0
[custom_ascii]
# color = "Green"
path = "~/.config/macchina/ascii/archlinux.ascii"
[randomize]
key_color = false
separator_color = false
[keys]
host = "Host"
kernel = "Kernel"
battery = "Battery"
os = "OS"
de = "DE"
wm = "WM"
distro = "Distro"
terminal = "Terminal"
shell = "Shell"
packages = "Packages"
uptime = "Uptime"
memory = "Memory"
machine = "Machine"
local_ip = "Local IP"
backlight = "Brightness"
resolution = "Resolution"
cpu_load = "CPU Load"
cpu = "CPU" Its this one. |
The theme parses and shows up correctly when building the usual EDIT: Nope, no drastic changes that would cause something like this to happen.
That would help us out a lot. |
Oh I forgot to mention, I figured it out. |
Good catch :) |
I actually hadn't pushed the #[serde(default)] changes. I'll merge them in #208 |
Woops, alright :) |
Added support for rgb and indexed colors by using cusom de/serialization of the
tui::style::Color
struct.Source here https://git.uttarayan.me/uttarayan/color-parser-tui. I will publish to crates.io after a bit more testing.