~tsdh/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
3 3

[PATCH swayr] swayrbar: add styling module config options

Details
Message ID
<ipms2hntn4dagws5bdxyqo23ziom5wsy4mddtirhxxalecexpb@udipokhodsgw>
DKIM signature
pass
Download raw message
Patch: +240 -90
Allow the following options to be passed directly to swaybar-protocol(7)

- align
- min_width
- color
- background
- border
- border_top
- border_bottom
- border_left
- border_right
- urgent
- separator
- separator_block_width
---
 While working on the cmd module I noticed there were a lot of `None`
 values passed to Block which would fit nicely as config options.

 On one hand, it gives configuration a bit of _style_ and makes them a
 bit fancy, on the other hand it means modules in the future will not be
 able to control the colors themselves dynamically. So for example the
 battery module could not change colors depending on the charge level.

 So this patch is also more of a suggestion than a direct feature
 request. It would be nice, but I can understand why it would not be
 desirable.

 I've tested all the config options and they work as expected and
 everything should be backwards compatible.

 I've built this on top of my cmd-module patch, but it should be easy to
 remove those changes if that one won't get applied first.

 PS: Rust is growing on me a bit :P

 README.md                      | 15 ++++++++++
 swayrbar/src/config.rs         | 50 ++++++++++++++++++++++++++++++++++
 swayrbar/src/module/battery.rs | 38 +++++++++++++++++---------
 swayrbar/src/module/cmd.rs     | 38 +++++++++++++++++---------
 swayrbar/src/module/date.rs    | 37 +++++++++++++++++--------
 swayrbar/src/module/pactl.rs   | 38 +++++++++++++++++---------
 swayrbar/src/module/sysinfo.rs | 38 +++++++++++++++++---------
 swayrbar/src/module/wifi.rs    | 38 +++++++++++++++++---------
 swayrbar/src/module/window.rs  | 38 +++++++++++++++++---------
 9 files changed, 240 insertions(+), 90 deletions(-)

diff --git a/README.md b/README.md
index 429b9e3..2632732 100644
--- a/README.md
+++ b/README.md
@@ -829,6 +829,21 @@ on_click = { Left = ['swayr', 'switch-to-urgent-or-lru-window'], Right = ['kill'

but then it has to be on one single line.

In addition the following options are available and get passed directly
to [`swaybar-protocol(7)`](https://man.archlinux.org/man/swaybar-protocol.7#BODY):

- `align`
- `min_width`
- `color`
- `background`
- `border`
- `border_top`
- `border_bottom`
- `border_left`
- `border_right`
- `urgent`
- `separator`
- `separator_block_width`

#### The `window` module

diff --git a/swayrbar/src/config.rs b/swayrbar/src/config.rs
index 013dd1a..6b811ce 100644
--- a/swayrbar/src/config.rs
+++ b/swayrbar/src/config.rs
@@ -19,6 +19,7 @@ use crate::module::BarModuleFn;
use crate::shared::cfg;
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use swaybar_types::{Align, Width};

#[derive(Debug, Serialize, Deserialize)]
pub struct Config {
@@ -36,6 +37,18 @@ pub struct ModuleConfig {
    pub format: String,
    pub html_escape: Option<bool>,
    pub on_click: Option<HashMap<String, Vec<String>>>,
    pub align: Option<String>,
    pub color: Option<String>,
    pub background: Option<String>,
    pub border: Option<String>,
    pub border_top: Option<u32>,
    pub border_bottom: Option<u32>,
    pub border_left: Option<u32>,
    pub border_right: Option<u32>,
    pub urgent: Option<bool>,
    pub separator: Option<bool>,
    pub separator_block_width: Option<u32>,
    pub min_width: Option<u32>,
}

impl ModuleConfig {
@@ -69,6 +82,28 @@ impl Default for Config {
    }
}

pub fn string_to_align(align: &Option<String>) -> Align {
    match align {
        Some(a) => match a.as_str() {
            "center" => Align::Center,
            "right" => Align::Right,
            "left" => Align::Left,
            _ => {
                log::warn!(
                "Couldn't match {0} to a valid Align value, defaulting to left",
                a,
            );
                Align::Left
            }
        },
        None => Align::Left,
    }
}

pub fn config_to_min_width(min_width: Option<u32>) -> Option<Width> {
    min_width.map(Width::Pixels)
}

pub fn load_config() -> Config {
    cfg::load_config::<Config>("swayrbar")
}
@@ -78,3 +113,18 @@ fn test_load_swayrbar_config() {
    let cfg = cfg::load_config::<Config>("swayrbar");
    println!("{:?}", cfg);
}

#[test]
fn test_string_to_align() {
    let left = string_to_align(&Some("left".to_owned()));
    let right = string_to_align(&Some("right".to_owned()));
    let center = string_to_align(&Some("center".to_owned()));
    let wrong = string_to_align(&Some("badbadbadbad".to_owned()));
    let default = string_to_align(&None);

    assert_eq!(left, Align::Left);
    assert_eq!(right, Align::Right);
    assert_eq!(center, Align::Center);
    assert_eq!(wrong, Align::Left);
    assert_eq!(default, Align::Left);
}
diff --git a/swayrbar/src/module/battery.rs b/swayrbar/src/module/battery.rs
index a10a344..30920fc 100644
--- a/swayrbar/src/module/battery.rs
+++ b/swayrbar/src/module/battery.rs
@@ -15,7 +15,7 @@

//! The battery `swayrbar` module.

use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use crate::shared::fmt::subst_placeholders;
use battery as bat;
@@ -123,6 +123,18 @@ impl BarModuleFn for BarModuleBattery {
            instance,
            format: "🔋 Bat: {state_of_charge:{:5.1}}%, {state}, Health: {state_of_health:{:5.1}}%".to_owned(),
            html_escape: Some(false),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: None,
        }
    }
@@ -146,20 +158,20 @@ impl BarModuleFn for BarModuleBattery {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/cmd.rs b/swayrbar/src/module/cmd.rs
index 1375f86..c2ad4cf 100644
--- a/swayrbar/src/module/cmd.rs
+++ b/swayrbar/src/module/cmd.rs
@@ -15,7 +15,7 @@

//! The cmd `swayrbar` module.

use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use std::process::Command;
use std::string::String;
@@ -63,6 +63,18 @@ impl BarModuleFn for BarModuleCmd {
            format: String::new(),
            html_escape: Some(true),
            on_click: None,
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
        }
    }

@@ -87,20 +99,20 @@ impl BarModuleFn for BarModuleCmd {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            min_width: config_to_min_width(self.config.min_width),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/date.rs b/swayrbar/src/module/date.rs
index a41d3e6..e6ccb64 100644
--- a/swayrbar/src/module/date.rs
+++ b/swayrbar/src/module/date.rs
@@ -17,6 +17,7 @@

use std::sync::Mutex;

use crate::config::{config_to_min_width, string_to_align};
use crate::module::config;
use crate::module::{BarModuleFn, RefreshReason};
use swaybar_types as s;
@@ -52,6 +53,18 @@ impl BarModuleFn for BarModuleDate {
            instance,
            format: "⏰ %F %X".to_owned(),
            html_escape: Some(false),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: None,
        }
    }
@@ -71,20 +84,20 @@ impl BarModuleFn for BarModuleDate {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/pactl.rs b/swayrbar/src/module/pactl.rs
index 8b46060..5c79ac9 100644
--- a/swayrbar/src/module/pactl.rs
+++ b/swayrbar/src/module/pactl.rs
@@ -15,7 +15,7 @@

//! The pactl `swayrbar` module.

use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use crate::shared::fmt::subst_placeholders;
use once_cell::sync::Lazy;
@@ -105,6 +105,18 @@ impl BarModuleFn for BarModulePactl {
            instance,
            format: "🔈 Vol: {volume:{:3}}%{muted}".to_owned(),
            html_escape: Some(true),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: Some(HashMap::from([
                ("Left".to_owned(), vec!["pavucontrol".to_owned()]),
                (
@@ -163,20 +175,20 @@ impl BarModuleFn for BarModulePactl {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/sysinfo.rs b/swayrbar/src/module/sysinfo.rs
index 35f4490..d215f8f 100644
--- a/swayrbar/src/module/sysinfo.rs
+++ b/swayrbar/src/module/sysinfo.rs
@@ -15,7 +15,7 @@

//! The sysinfo `swayrbar` module.

use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use crate::shared::fmt::subst_placeholders;
use std::collections::HashMap;
@@ -136,6 +136,18 @@ impl BarModuleFn for BarModuleSysInfo {
            instance,
            format: "💻 CPU: {cpu_usage:{:5.1}}% Mem: {mem_usage:{:5.1}}% Load: {load_avg_1:{:5.2}} / {load_avg_5:{:5.2}} / {load_avg_15:{:5.2}}".to_owned(),
            html_escape: Some(false),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: Some(HashMap::from([
               ("Left".to_owned(),
                vec!["foot".to_owned(), "htop".to_owned()])])),
@@ -163,20 +175,20 @@ impl BarModuleFn for BarModuleSysInfo {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/wifi.rs b/swayrbar/src/module/wifi.rs
index 405a756..45ecfb5 100644
--- a/swayrbar/src/module/wifi.rs
+++ b/swayrbar/src/module/wifi.rs
@@ -1,4 +1,4 @@
use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use crate::shared::fmt::subst_placeholders;
use once_cell::sync::Lazy;
@@ -188,6 +188,18 @@ impl BarModuleFn for BarModuleWifi {
            instance,
            format: "📡 Wi-fi: {name}{bars}{signal}".to_owned(),
            html_escape: Some(false),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: None,
        }
    }
@@ -212,20 +224,20 @@ impl BarModuleFn for BarModuleWifi {
            name: Some(self.tool.to_string()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.to_owned(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

diff --git a/swayrbar/src/module/window.rs b/swayrbar/src/module/window.rs
index e9c6fd0..23b2c16 100644
--- a/swayrbar/src/module/window.rs
+++ b/swayrbar/src/module/window.rs
@@ -19,7 +19,7 @@ use std::collections::HashMap;
use std::sync::Mutex;
use std::time::{Duration, Instant};

use crate::config;
use crate::config::{self, config_to_min_width, string_to_align};
use crate::module::{BarModuleFn, RefreshReason};
use crate::shared::fmt::subst_placeholders;
use crate::shared::ipc;
@@ -115,6 +115,18 @@ impl BarModuleFn for BarModuleWindow {
            instance,
            format: "🪟 {title} — {app_name}".to_owned(),
            html_escape: Some(false),
            align: Some("left".to_owned()),
            color: None,
            border: None,
            urgent: None,
            separator: None,
            background: None,
            border_top: None,
            border_left: None,
            border_right: None,
            border_bottom: None,
            separator_block_width: None,
            min_width: None,
            on_click: Some(HashMap::from([
                (
                    "Left".to_owned(),
@@ -192,20 +204,20 @@ impl BarModuleFn for BarModuleWindow {
            name: Some(NAME.to_owned()),
            instance: Some(self.config.instance.clone()),
            full_text: state.cached_text.clone(),
            align: Some(s::Align::Left),
            align: Some(string_to_align(&self.config.align)),
            markup: Some(s::Markup::Pango),
            short_text: None,
            color: None,
            background: None,
            border: None,
            border_top: None,
            border_bottom: None,
            border_left: None,
            border_right: None,
            min_width: None,
            urgent: None,
            separator: Some(true),
            separator_block_width: None,
            min_width: config_to_min_width(self.config.min_width),
            color: self.config.color.clone(),
            background: self.config.background.clone(),
            border: self.config.border.clone(),
            border_top: self.config.border_top,
            border_bottom: self.config.border_bottom,
            border_left: self.config.border_left,
            border_right: self.config.border_right,
            urgent: self.config.urgent,
            separator: self.config.separator,
            separator_block_width: self.config.separator_block_width,
        }
    }

-- 
2.45.2

[swayr/patches/arch.yml] build failed

builds.sr.ht <builds@sr.ht>
Details
Message ID
<D2NNB9EIU2M5.2VPFDOGED4LDW@fra01>
In-Reply-To
<ipms2hntn4dagws5bdxyqo23ziom5wsy4mddtirhxxalecexpb@udipokhodsgw> (view parent)
DKIM signature
missing
Download raw message
swayr/patches/arch.yml: FAILED in 23s

[swayrbar: add styling module config options][0] from [Luca Matei Pintilie][1]

[0]: https://lists.sr.ht/~tsdh/public-inbox/patches/53829
[1]: luca@lucamatei.com

✗ #1274486 FAILED swayr/patches/arch.yml https://builds.sr.ht/~tsdh/job/1274486
Details
Message ID
<87zfqi7lp2.fsf@gnu.org>
In-Reply-To
<ipms2hntn4dagws5bdxyqo23ziom5wsy4mddtirhxxalecexpb@udipokhodsgw> (view parent)
DKIM signature
pass
Download raw message
Luca Matei Pintilie <luca@lucamatei.com> writes:

Hi Luca,

>  On one hand, it gives configuration a bit of _style_ and makes them a
>  bit fancy, on the other hand it means modules in the future will not
>  be able to control the colors themselves dynamically. So for example
>  the battery module could not change colors depending on the charge
>  level.

Indeed.  That's a complication and tradeoff which I have dealt with for
now.  For styling, alignment, and padding one can already use string
formatting in `format` which works ok assuming one uses a proportional
font in swaybar.  And one can use pango (basic HTML) formatting, too,
which allows for colorization, I think.

So making all swaybar properties configurable would probably just add
another way to add styling.  But ok, right now there's no way to affect
the borders...

Wrt. dynamically changing the styling based on things like battery
level: that's actually a good idea in theory and could be added right
now where it makes sense (like the battery module).  However, it could
probably look bad given that we have no clue about the general swaybar
styling, i.e., a "warning style" for a dark blue swaybar has probably a
different look than a warning style for a silverish, transparent
swaybar.

So all in all, I'm quite unsure what to do and would therefore simply do
nothing.  Maybe you could convince me in favour of your patch with a
screenshot of your nicely styled swaybar...

If we'd apply your patch, we'd probably also want to deprecate the
html_escape config because one could then set markup to "none" which has
the same effect.  I have this option only because pango markup is
currently always enabled.

Bye,
  Tassilo

PS: min_width is either an integer (pixels) or a string (sample text
from which the min width is computed).  In your patch, it's always an
integer.  I think specifying a fixed pixel width is not a good option
because many people work on laptops and connect them to monitors of
different sizes so what's a good config for one workplace might be not
suited for the other one.
Details
Message ID
<suo6xcz7crpou3m52ofcluatqpbzz6sx6n5uhf64zjgvekth43@7pud77qsjqh2>
In-Reply-To
<87zfqi7lp2.fsf@gnu.org> (view parent)
DKIM signature
pass
Download raw message
On Mon, Jul 15, 2024 at 04:51:21PM GMT, Tassilo Horn wrote:
>And one can use pango (basic HTML) formatting, too, which allows for
>colorization, I think.
>
>So making all swaybar properties configurable would probably just add
>another way to add styling.  But ok, right now there's no way to affect
>the borders...

I was not aware of that, but I've taken a look and I'm not impressed.

>Wrt. dynamically changing the styling based on things like battery
>level: that's actually a good idea in theory and could be added right
>now where it makes sense (like the battery module).  However, it could
>probably look bad given that we have no clue about the general swaybar
>styling, i.e., a "warning style" for a dark blue swaybar has probably a
>different look than a warning style for a silverish, transparent
>swaybar.

Actually this might be a good fit for pango and the cmd module if it
looked better. There are some options from here, such as adding
"themes", but that might be a bigger change than I had originally
planned.

>So all in all, I'm quite unsure what to do and would therefore simply do
>nothing.  Maybe you could convince me in favour of your patch with a
>screenshot of your nicely styled swaybar...

That is perfectly resonnable. I just intended this to be a suggestion,
and wanted a bit of color at the top.

I'm not much of a ricer, but I wanted to sprinkle in some dracula themed
colors to match everything else I have in my environment, and I think it
looks not half bad ;)

https://lucamatei.com/paste/94b96cc9-a3ff-4907-adb6-f989407ab6ef.png

>If we'd apply your patch, we'd probably also want to deprecate the
>html_escape config because one could then set markup to "none" which has
>the same effect.  I have this option only because pango markup is
>currently always enabled.

Yeah, I didn't want to touch anything that was in use right now because
of backwards compatibility.

>PS: min_width is either an integer (pixels) or a string (sample text
>from which the min width is computed).  In your patch, it's always an
>integer.  I think specifying a fixed pixel width is not a good option
>because many people work on laptops and connect them to monitors of
>different sizes so what's a good config for one workplace might be not
>suited for the other one.

That's a great point which I hadn't thought about. I'm not very sure how
it works with rust, toml, and config options that can have multiple
types, but that is certainly something that can be investigated further.

Thanks for all the feedback! I'll leave this patch as is for now, and
won't continue it any further.

--
Luca
Reply to thread Export thread (mbox)