Skip to content

Expose allDesktops option from rc.xml in the desktop section #253

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jamieosullivan
Copy link

@jamieosullivan jamieosullivan commented Mar 27, 2025

See issue #252

Some UX and technical aspects complicate things. The "allDesktops" option is fairly buried in the keybindings in the xml, hence the strings for the node names. I guess there's potential for something to break if some other program is used for remapping those.

There seems to be a tension between the separation of concerns between the window manager and desktop environment. Also, whether this belongs in the "Desktops" section of obconf-qt, the "Windows" section, or a separate section, I'm not altogther sure.

@@ -167,6 +173,11 @@ void MainDialog::on_desktop_popup_toggled(bool checked) {
tree_set_int("desktops/popupTime", 0);
}

void MainDialog::on_all_desktops_toggled(bool checked) {
tree_set_bool(all_desktops_node_1, checked);
tree_set_bool(all_desktops_node_2, checked);
Copy link
Author

Choose a reason for hiding this comment

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

I realise it's a little bit suboptimal having 2 calls to tree_apply() via these functions, maybe another function to avoid that is merited

@@ -33,6 +33,9 @@ extern RrInstance* rrinst; // defined in obconf-qt.cpp

static int num_desktops;

static const gchar* all_desktops_node_1 = "keyboard/keybind:key=A-Tab/action:name=NextWindow/allDesktops";
static const gchar* all_desktops_node_2 = "keyboard/keybind:key=A-S-Tab/action:name=PreviousWindow/allDesktops";
Copy link
Author

Choose a reason for hiding this comment

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

These long names are a bit ungainly and potentially fragile because of where the allDesktops entries are nested in the xml.

Ideally (IMO) allDesktops would be broken out of the keybinds to some more convenient and stable location, as the behaviour seems independent of them to me, but Openbox seems to be mature and unchanged for a long time so I guess that's unlikely to happen. Doubtless there are a lot of programs out there using the current configurations that any change would break.

Could maybe do something a bit more comprehensive here, like search the keybinds to see which ones have <action name="[Next|Previous]Window"> or allDesktops and fill into a template string such as "keyboard/keybind:key=%s/action:name=%s/allDesktops" with ``g_strdup_printf`. Just means a bit more processing and code, and i'm conscious of introducing bugs.

@jamieosullivan
Copy link
Author

Quick clip of it in action

allDesktops_toggle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant