Make some termwiz deps optional #144
Conversation
|
|
||
| pub fn to_linear_tuple_rgba(self) -> RgbaTuple { | ||
| self.to_linear().into_components() | ||
| ( |
There was a problem hiding this comment.
This is the one thing I'm unsure about in this PR; there are some subtle differences between SRGB and RGB to do with the non-linear scaling in SRGB. I think this change is good, but it's worth making sure that the old implementation was just doing simple divide by 255; the consequence of this not being right is that the colors will have the wrong brightness. This is more obvious with brighter color schemes!
There was a problem hiding this comment.
I kept both functions, although this to_linear one does not seem to be used.
termwiz/src/color.rs
Outdated
| let color = Srgb::<u8>::from_format(color); | ||
| Self::new(color.red, color.green, color.blue) | ||
| }) | ||
| #[cfg(feature = "color-names")] |
There was a problem hiding this comment.
I think I'd like to replace this with a lazy-static hashmap initialized from rgb.txt.
That file comes originally from X11, but vim also has a decent copy:
$ wc -l /usr/share/vim/vim82/rgb.txt
782 /usr/share/vim/vim82/rgb.txt
$ head /usr/share/vim/vim82/rgb.txt
255 250 250 snow
248 248 255 ghost white
248 248 255 GhostWhite
245 245 245 white smoke
245 245 245 WhiteSmoke
220 220 220 gainsboro
255 250 240 floral white
255 250 240 FloralWhite
253 245 230 old lace
253 245 230 OldLace
it should hopefully be pretty trivial to include_str! that file and build up a map of the lowercased name -> rgb values for use here
There was a problem hiding this comment.
I added an rgb.txt based alternative to the palette crate for this case, so we shouldn't need the color names feature in this PR
| /// The contents of the TERM environment variable | ||
| term: Option<String>, | ||
| builder! { | ||
| /// Use the `ProbeHints` to configure an instance of |
There was a problem hiding this comment.
Will Rust let us do something like:
#[deprecated("Use ProbeHints directly")]
pub type ProbeHintsBuilder = ProbeHints;
to help guide anyone that might be using the hint builder? I don't think this is critical and it may only be streampager that was using this anyway; if we can't deprecate the use of a type alias then we don't need to worry about it.
There was a problem hiding this comment.
I looked at the generated code and didn't quite like the API:
pub struct ProbeHintsBuilder {
#[doc = " The contents of the TERM environment variable"]
term: ::std::option::Option<Option<String>>,
...
}
impl ProbeHintsBuilder {
#[doc = " The contents of the TERM environment variable"]
#[allow(unused_mut)]
pub fn term(&mut self, value: Option<String>) -> &mut Self {
let mut new = self;
new.term = ::std::option::Option::Some(value);
new
}
pub fn build(&self)
-> ::std::result::Result<ProbeHints, ::std::string::String> {
...
}
}
I'd like to get rid of the double Option, and make build infalliable (or not needed). So the API is incompatible now.
If we want compatibility I can restore the old API.
Embed rgb.txt and parse it on the fly to produce the list of colors. This list is a superset of palette's SVG color list. refs: #144
The palette crate has a codegen step that translates svg_colors.txt to named.rs.
That makes it hard to build using buck.
Remove the palette dependency so termwiz is easier to build using buck.
I made sure the following code:
fn main() {
use termwiz::color::RgbColor;
let r = RgbColor::from_rgb_str("#02abcd").unwrap();
let r1 = r.to_tuple_rgba();
let r2 = r.to_linear_tuple_rgba();
println!("r1 = {:?}", r1);
println!("r2 = {:?}", r2);
}
prints
r1 = (0.007843138, 0.67058825, 0.8039216, 1.0)
r2 = (0.000607054, 0.4072403, 0.6104956, 1.0)
before and after the change.
derive_builder has some extra dependencies that take a while to compile. The builder feature can be expressed via a 30-line macro. So let's do that to make termwiz compile faster.
This makes termwiz (likely) use the same regex when being compiled together with other dependencies in the eco-system.
quark-zju
force-pushed
the
termwiz-deps
branch
from
7bfcd05
to
a7872d0
Compare
The main issue is
palette, which does not build with buck. I also replacedderive_builderwith an ad-hoc macro.