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.

Sign up for GitHub

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 percent in config values for height and width #51

Closed
wants to merge 1 commit into from
Closed

Support percent in config values for height and width #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ Sample configuration:
# This file should be placed at ~/.config/yofi/yofi.config

# ~~Global values, used as fallback if needed
width = 400
height = 512
width = "400"
height = "512"
# If set forces usage of common window instead of Layer Shell protocol
force_window = false
# if unset, places window at the center
Expand Down
4 changes: 2 additions & 2 deletions src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ fn deserialize_color<'de, D: serde::Deserializer<'de>>(d: D) -> Result<u32, D::E

#[derive(Default, Deserialize)]
pub struct Config {
width: Option<u32>,
height: Option<u32>,
width: Option<String>,
height: Option<String>,
force_window: Option<bool>,
window_offsets: Option<(i32, i32)>,
scale: Option<u16>,
Expand Down
18 changes: 16 additions & 2 deletions src/config/params.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ impl<'a> From<&'a Config> for BgParams {
impl<'a> From<&'a Config> for SurfaceParams {
fn from(config: &'a Config) -> SurfaceParams {
SurfaceParams {
width: config.width.unwrap_or(400),
height: config.height.unwrap_or(512),
width: parse_cent(config.width.as_ref(), 400),
height: parse_cent(config.height.as_ref(), 512),
force_window: config.force_window.unwrap_or(false),
window_offsets: config.window_offsets,
scale: config.scale,
Expand Down Expand Up @@ -140,3 +140,17 @@ fn color_to_solid_source(x: Color) -> SolidSource {
let bytes = x.to_be_bytes();
SolidSource::from_unpremultiplied_argb(bytes[3], bytes[0], bytes[1], bytes[2])
}

fn parse_cent(input: Option<&String>, default: u32) -> u32 {
let input: String = match input {
Some(a) => a.split_whitespace().collect(),
None => return default,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still may return Option-s everywhere and at SurfaceParams call unwrap_or as before.

};
if input.contains('%') {
return match input.trim().strip_suffix('%').unwrap().parse::<u32>() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. No need in trim there and below
  2. strip_prefix returns Option<&str> without specified suffix, so you don't need contains call above

Ok(a) => (a * default) / 100,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Err(_) => default,
};
}
input.trim().parse::<u32>().unwrap_or(default)
}