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

Add more specific margin capabilities #196

Merged
merged 3 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions src/config/theme_setting.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::errors::Result;
use crate::models::Margins;
use serde::{Deserialize, Serialize};
use std::default::Default;
use std::fs;
Expand All @@ -7,8 +8,8 @@ use std::path::Path;
#[serde(default)]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct ThemeSetting {
pub border_width: u32,
pub margin: u32,
pub border_width: i32,
pub margin: Margins,
pub default_border_color: String,
pub floating_border_color: String,
pub focused_border_color: String,
Expand All @@ -32,7 +33,7 @@ impl Default for ThemeSetting {
fn default() -> Self {
ThemeSetting {
border_width: 1,
margin: 10,
margin: Margins::Int(10),
default_border_color: "#000000".to_owned(),
floating_border_color: "#000000".to_owned(),
focused_border_color: "#FF0000".to_owned(),
Expand Down Expand Up @@ -70,7 +71,7 @@ on_new_window = 'echo Hello World'
config,
ThemeSetting {
border_width: 0,
margin: 5,
margin: Margins::Int(5),
default_border_color: "#222222".to_string(),
floating_border_color: "#005500".to_string(),
focused_border_color: "#FFB53A".to_string(),
Expand Down
4 changes: 2 additions & 2 deletions src/layouts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl Layout {
#[cfg(test)]
mod tests {
use super::*;
use crate::models::{BBox, WindowHandle};
use crate::models::{BBox, Margins, WindowHandle};

#[test]
fn should_fullscreen_a_single_window() {
Expand All @@ -114,7 +114,7 @@ mod tests {
ws.update_avoided_areas();
let mut w = Window::new(WindowHandle::MockHandle(1), None);
w.border = 0;
w.margin = 0;
w.margin = Margins::Int(0);
let mut windows = vec![&mut w];
let mut windows_filters = windows.iter_mut().filter(|_f| true).collect();
even_horizontal::update(&ws, &mut windows_filters);
Expand Down
43 changes: 43 additions & 0 deletions src/models/margins.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
use serde::{Deserialize, Serialize};

#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(untagged)]
pub enum Margins {
Int(u32),
Vec(Vec<u32>),
} // format: [top, right, bottom, left] as per HTML

impl Margins {
pub fn left(self) -> i32 {
self.into_vec()[3] as i32
}
pub fn right(self) -> i32 {
self.into_vec()[1] as i32
}
pub fn top(self) -> i32 {
self.into_vec()[0] as i32
}
pub fn bottom(self) -> i32 {
self.into_vec()[2] as i32
}

pub fn into_vec(self) -> Vec<u32> {
match self {
Self::Vec(v) => match v.len() {
1 => vec![v[0], v[0], v[0], v[0]],
2 => vec![v[0], v[0], v[1], v[1]],
3 => vec![v[0], v[1], v[2], v[2]],
4 => v,
0 => {
log::error!("Empty margin or border array");
vec![10, 10, 10, 10] //assume 5 px borders for now
}
_ => {
log::error!("Too many entries in margin or border array");
vec![v[0], v[1], v[2], v[3]] //assume later entries are invalid
}
},
Self::Int(x) => vec![x, x, x, x],
}
}
}
2 changes: 2 additions & 0 deletions src/models/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod dock_area;
mod manager;
mod margins;
mod mode;
mod screen;
mod tag;
Expand All @@ -16,6 +17,7 @@ use crate::layouts;

pub use dock_area::DockArea;
pub use manager::Manager;
pub use margins::Margins;
pub use mode::Mode;
pub use screen::{BBox, Screen};
pub use window::Window;
Expand Down
35 changes: 22 additions & 13 deletions src/models/window.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::WindowState;
use super::WindowType;
use crate::config::ThemeSetting;
use crate::models::Margins;
use crate::models::TagId;
use crate::models::XYHWBuilder;
use crate::models::XYHW;
Expand Down Expand Up @@ -28,7 +29,7 @@ pub struct Window {
pub type_: WindowType,
pub tags: Vec<TagId>,
pub border: i32,
pub margin: i32,
pub margin: Margins,
states: Vec<WindowState>,
pub normal: XYHW,
pub start_loc: Option<XYHW>,
Expand All @@ -48,7 +49,7 @@ impl Window {
type_: WindowType::Normal,
tags: Vec::new(),
border: 1,
margin: 10,
margin: Margins::Int(10),
states: vec![],
normal: XYHWBuilder::default().into(),
floating: None,
Expand All @@ -59,10 +60,10 @@ impl Window {

pub fn update_for_theme(&mut self, theme: &ThemeSetting) {
if self.type_ == WindowType::Normal {
self.margin = theme.margin as i32;
self.border = theme.border_width as i32;
self.margin = theme.margin.clone();
self.border = theme.border_width;
} else {
self.margin = 0;
self.margin = Margins::Int(0);
self.border = 0;
}
}
Expand Down Expand Up @@ -147,9 +148,13 @@ impl Window {
value = self.normal.w();
} else if self.floating() && self.floating.is_some() {
let relative = self.normal + self.floating.unwrap();
value = relative.w() - (self.margin * 2) - (self.border * 2);
value = relative.w()
- (self.margin.clone().left() + self.margin.clone().right())
Copy link
Member

Choose a reason for hiding this comment

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

@mautamu or @lex148: may I ask, why you apply a margin to a floating window?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m not sure exactly why it’s applied. Can try running it without and see what happens.

Copy link
Member

Choose a reason for hiding this comment

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

Might test myself as well. Was just curious if there was some reasoning behind that.

- (self.border * 2);
} else {
value = self.normal.w() - (self.margin * 2) - (self.border * 2);
value = self.normal.w()
- (self.margin.clone().left() + self.margin.clone().right())
- (self.border * 2);
}
if value < 100 && self.type_ != WindowType::Dock {
value = 100
Expand All @@ -162,9 +167,13 @@ impl Window {
value = self.normal.h();
} else if self.floating() && self.floating.is_some() {
let relative = self.normal + self.floating.unwrap();
value = relative.h() - (self.margin * 2) - (self.border * 2);
value = relative.h()
- (self.margin.clone().top() + self.margin.clone().bottom())
- (self.border * 2);
} else {
value = self.normal.h() - (self.margin * 2) - (self.border * 2);
value = self.normal.h()
- (self.margin.clone().top() + self.margin.clone().bottom())
- (self.border * 2);
}
if value < 100 && self.type_ != WindowType::Dock {
value = 100
Expand Down Expand Up @@ -193,9 +202,9 @@ impl Window {
}
if self.floating() && self.floating.is_some() {
let relative = self.normal + self.floating.unwrap();
relative.x() + self.margin
relative.x() + self.margin.clone().left()
} else {
self.normal.x() + self.margin
self.normal.x() + self.margin.clone().left()
}
}

Expand All @@ -205,9 +214,9 @@ impl Window {
}
if self.floating() && self.floating.is_some() {
let relative = self.normal + self.floating.unwrap();
relative.y() + self.margin
relative.y() + self.margin.clone().bottom()
} else {
self.normal.y() + self.margin
self.normal.y() + self.margin.clone().bottom()
Copy link
Member

@VuiMuich VuiMuich Mar 28, 2021

Choose a reason for hiding this comment

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

another question if I may: why are you using .bottom() here and not .top()? Isn't the origin for determining window-positions the top-left screen corner?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that you mention it, that’s probably right. This might be an artifact from when I was confused and had the HTML top/bottom/ left/right specs backwards.

Should probably check over and see if the behaviour is what we’re expecting it to be.

Copy link
Member

@VuiMuich VuiMuich Mar 29, 2021

Choose a reason for hiding this comment

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

as I am just about to get a MarginMultiplier PR ready (just struggling to get the external command working) so I might include this..

Copy link
Member Author

Choose a reason for hiding this comment

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

That sounds great! Thanks for reviewing that. I wonder if it's partially related to #248

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if it's partially related to #248

Gonna look into this tomorrow, now it's bedtime. Gn8

}
}

Expand Down
4 changes: 2 additions & 2 deletions src/models/window_change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::Window;
use super::WindowHandle;
use super::WindowState;
use super::WindowType;
use crate::models::XYHWChange;
use crate::models::{Margins, XYHWChange};

type MaybeWindowHandle = Option<WindowHandle>;
type MaybeName = Option<String>;
Expand Down Expand Up @@ -82,7 +82,7 @@ impl WindowChange {
window.type_ = type_.clone();
if window.type_ == WindowType::Dock {
window.border = 0;
window.margin = 0;
window.margin = Margins::Int(0);
}
}
if let Some(states) = self.states {
Expand Down