Support percent in config values for height and width #51
Conversation
| }; | ||
| if input.contains('%') { | ||
| return match input.trim().strip_suffix('%').unwrap().parse::<u32>() { | ||
| Ok(a) => (a * default) / 100, |
There was a problem hiding this comment.
Unfortunately that won't work, current value is interpreted as pixels. Thus 80% will be converted to 80px (instead of expected 0.8 * screen_side).
Instead I'll suggest to create a separate type for width/height, like this:
enum SideSize {
Absolute { pixels: u32 },
Relative { ratio: f32 },
}Then at the surface.rs SideSize::Relative should be converted into the absolute value.
There was a problem hiding this comment.
I was thinking relative to the default values, So 80% would be converted to 320px for height.
I seem to have missed the intent.
I will try this new way.
There was a problem hiding this comment.
Indeed, some of issues are vaguely described, mostly because these are written for myself. Sorry for inconvenience, feel free to ask any question, if anything else is ambiguous.
| None => return default, | ||
| }; | ||
| if input.contains('%') { | ||
| return match input.trim().strip_suffix('%').unwrap().parse::<u32>() { |
There was a problem hiding this comment.
- No need in
trimthere and below strip_prefixreturnsOption<&str>without specified suffix, so you don't needcontainscall above
| fn parse_cent(input: Option<&String>, default: u32) -> u32 { | ||
| let input: String = match input { | ||
| Some(a) => a.split_whitespace().collect(), | ||
| None => return default, |
There was a problem hiding this comment.
You still may return Option-s everywhere and at SurfaceParams call unwrap_or as before.
|
There's no activity there for more than a year so I'm closing this. Feel free to open a new PR or update this one |
I was unsure about where to put the function.
config/params.rsseemed like the right place.Resolves #25