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

feat: icon overrides #837

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
3 changes: 2 additions & 1 deletion docs/Configuration guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ The following table lists each of the bar-level bar config options:
| `exclusive_zone` | `boolean` | `true` unless `start_hidden` is enabled. | Whether the bar should reserve an exclusive zone around it. |
| `popup_gap` | `integer` | `5` | The gap between the bar and popup window. |
| `icon_theme` | `string` | `null` | Name of the GTK icon theme to use. Leave blank to use default. |
| `icon_overrides` | `Map<string, string>` | `{}` | Map of app IDs (or classes) to icon names, overriding the app's default icon. |
| `start_hidden` | `boolean` | `false`, or `true` if `autohide` set | Whether the bar should be hidden when the application starts. Enabled by default when `autohide` is set. |
| `autohide` | `integer` | `null` | The duration in milliseconds before the bar is hidden after the cursor leaves. Leave unset to disable auto-hide behaviour. |
| `start` | `Module[]` | `[]` | Array of left or top modules. |
Expand Down Expand Up @@ -353,4 +354,4 @@ For information on the `Script` type, and embedding scripts in strings, see [her
| `name` | `string` | `null` | Sets the unique widget name, allowing you to style it using `#name`. |
| `class` | `string` | `null` | Sets one or more CSS classes, allowing you to style it using `.class`. |

For more information on styling, please see the [styling guide](styling-guide).
For more information on styling, please see the [styling guide](styling-guide).
2 changes: 1 addition & 1 deletion docs/modules/Launcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Optionally displays a launchable set of favourites.

| | Type | Default | Description |
|-----------------------------|---------------------------------------------|----------|--------------------------------------------------------------------------------------------------------------------------|
| `favorites` | `string[]` | `[]` | List of app IDs (or classes) to always show at the start of the launcher |
| `favorites` | `string[]` | `[]` | List of app IDs (or classes) to always show at the start of the launcher. |
| `show_names` | `boolean` | `false` | Whether to show app names on the button label. Names will still show on tooltips when set to false. |
| `show_icons` | `boolean` | `true` | Whether to show app icons on the button. |
| `icon_size` | `integer` | `32` | Size to render icon at (image icons only). |
Expand Down
1 change: 1 addition & 0 deletions src/bar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl Bar {
output_name: &self.monitor_name,
location: $location,
icon_theme: &icon_theme,
icon_overrides: &config.icon_overrides,
}
};
}
Expand Down
8 changes: 8 additions & 0 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,13 @@ pub struct BarConfig {
/// **Default**: `null`
pub icon_theme: Option<String>,

/// Map of app IDs (or classes) to icon names,
/// overriding the app's default icon.
///
/// **Default**; `{}`
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// **Default**; `{}`
/// **Default**: `{}`

#[serde(default)]
pub icon_overrides: HashMap<String, String>,
Copy link
Owner

Choose a reason for hiding this comment

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

This gives the ability to set overrides per-bar. I'm not sure I can see a use-case for that personally - it would probably be better to have this on the Config struct so that it applies application-wide.

Happy to keep as-is if you think otherwise though?


/// An array of modules to append to the start of the bar.
/// Depending on the orientation, this is either the top of the left edge.
///
Expand Down Expand Up @@ -338,6 +345,7 @@ impl Default for BarConfig {
start_hidden: None,
autohide: None,
icon_theme: None,
icon_overrides: HashMap::default(),
start: Some(vec![ModuleConfig::Label(
LabelModule::new("ℹ️ Using default config".to_string()).into(),
)]),
Expand Down
9 changes: 8 additions & 1 deletion src/modules/focused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ impl Module<gtk::Box> for FocusedModule {
info: &ModuleInfo,
) -> Result<ModuleParts<gtk::Box>> {
let icon_theme = info.icon_theme;
let icon_overrides = info.icon_overrides;

let container = gtk::Box::new(info.bar_position.orientation(), 5);

Expand All @@ -153,9 +154,15 @@ impl Module<gtk::Box> for FocusedModule {

{
let icon_theme = icon_theme.clone();
let icon_overrides = icon_overrides.clone();
Copy link
Owner

Choose a reason for hiding this comment

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

If this were to move to Config (see other comment)...I don't particularly like that we need to clone here, as it'll re-alloc for every focused/launcher instance. It'd be cheaper to wrap in an Arc (no Mutex needed since we're not mutating).

If it's kept at bar-level config though, I think this is fine since in most cases you'll only have 1 copy of the launcher or focused module at a time anyway.

Copy link
Contributor Author

@BowDown097 BowDown097 Feb 5, 2025

Choose a reason for hiding this comment

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

I'm not sure how icon_overrides could be used in the closure without performing a clone (letalone in an Arc). Wouldn't there be lifetime issues with info, or with a borrow of context.ironbar.config outside the closure?

Copy link
Owner

@JakeStanger JakeStanger Feb 5, 2025

Choose a reason for hiding this comment

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

You'd still need to clone, but if it were in an Arc, the clone would just increase a reference count rather than re-alloc so is (potentially) much cheaper.

Copy link
Owner

Choose a reason for hiding this comment

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

I meant put the Arc on the ModuleInfo struct when it's first created, instead of the reference. You can then freely clone the Arc. The current implementation still fully clones the data, then puts the cloned result in an Arc that's used once.

glib_recv!(context.subscribe(), data => {
if let Some((name, id)) = data {
if let Some((name, mut id)) = data {
if self.show_icon {

if let Some(icon) = icon_overrides.get(&id) {
id = icon.clone();
}

match ImageProvider::parse(&id, &icon_theme, true, self.icon_size)
.map(|image| image.load_into_image(&icon))
{
Expand Down
14 changes: 12 additions & 2 deletions src/modules/launcher/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,23 @@ pub struct Item {
pub open_state: OpenState,
pub windows: IndexMap<usize, Window>,
pub name: String,
pub icon_override: String,
}

impl Item {
pub fn new(app_id: String, open_state: OpenState, favorite: bool) -> Self {
pub fn new(
app_id: String,
icon_override: String,
open_state: OpenState,
favorite: bool,
) -> Self {
Self {
app_id,
favorite,
open_state,
windows: IndexMap::new(),
name: String::new(),
icon_override,
}
}

Expand Down Expand Up @@ -108,6 +115,7 @@ impl From<ToplevelInfo> for Item {
open_state,
windows,
name,
icon_override: String::new(),
}
}
}
Expand Down Expand Up @@ -167,7 +175,9 @@ impl ItemButton {
}

if appearance.show_icons {
let input = if item.app_id.is_empty() {
let input = if !item.icon_override.is_empty() {
item.icon_override.clone()
} else if item.app_id.is_empty() {
item.name.clone()
} else {
item.app_id.clone()
Expand Down
30 changes: 25 additions & 5 deletions src/modules/launcher/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ impl Module<gtk::Box> for LauncherModule {

fn spawn_controller(
&self,
_info: &ModuleInfo,
info: &ModuleInfo,
context: &WidgetContext<Self::SendMessage, Self::ReceiveMessage>,
mut rx: mpsc::Receiver<Self::ReceiveMessage>,
) -> crate::Result<()> {
Expand All @@ -149,24 +149,32 @@ impl Module<gtk::Box> for LauncherModule {
favorites
.iter()
.map(|app_id| {
let icon_override = info
.icon_overrides
.get(app_id)
.map_or_else(String::new, |v| v.to_string());

(
app_id.to_string(),
Item::new(app_id.to_string(), OpenState::Closed, true),
Item::new(app_id.to_string(), icon_override, OpenState::Closed, true),
)
})
.collect::<IndexMap<_, _>>()
});

let items = arc_mut!(items);

let items2 = Arc::clone(&items);

let icon_overrides = arc_mut!(info.icon_overrides.clone());
Copy link
Owner

Choose a reason for hiding this comment

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

This does not need to be mutable. Removing the mutex will also simplify some of the code below as it removes the need to lock.

let icon_overrides2 = Arc::clone(&icon_overrides);

let tx = context.tx.clone();
let tx2 = context.tx.clone();

let wl = context.client::<wayland::Client>();
spawn(async move {
let items = items2;
let icon_overrides = icon_overrides2;
let tx = tx2;

let mut wlrx = wl.subscribe_toplevels();
Expand All @@ -180,7 +188,14 @@ impl Module<gtk::Box> for LauncherModule {
item.merge_toplevel(info.clone());
}
None => {
items.insert(info.app_id.clone(), Item::from(info.clone()));
let mut item = Item::from(info.clone());
let icon_overrides = lock!(icon_overrides);

if let Some(icon) = icon_overrides.get(&info.app_id) {
item.icon_override = icon.clone();
}

items.insert(info.app_id.clone(), item);
}
}
}
Expand Down Expand Up @@ -210,7 +225,12 @@ impl Module<gtk::Box> for LauncherModule {
let item = items.get_mut(&info.app_id);
match item {
None => {
let item: Item = info.into();
let mut item: Item = info.into();
let icon_overrides = lock!(icon_overrides);

if let Some(icon) = icon_overrides.get(&app_id) {
item.icon_override = icon.clone();
}

items.insert(app_id.clone(), item.clone());

Expand Down
2 changes: 2 additions & 0 deletions src/modules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::HashMap;
use std::fmt::Debug;
use std::rc::Rc;
use std::sync::Arc;
Expand Down Expand Up @@ -71,6 +72,7 @@ pub struct ModuleInfo<'a> {
pub monitor: &'a Monitor,
pub output_name: &'a str,
pub icon_theme: &'a IconTheme,
pub icon_overrides: &'a HashMap<String, String>,
}

#[derive(Debug, Clone)]
Expand Down