! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
add a way to run a command when clicking on systemload plugin


Description Yves-Alexis Perez editbugs 2008-02-28 09:39:42 CET
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en; rv: Gecko/20070718 Fedora/ Epiphany/2.16 Firefox/
Build Identifier: 

A debian user requested [0] some way to run xfce4-taskmanager when clicking on systemload plugin. Maybe adding a configurable "run command on click" would be nice.


[0]: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=417998

Reproducible: Always
Comment 1 liquider 2014-01-26 17:46:25 CET
Created attachment 5336 
run configurable command on click or context menu activation

Attached is a patch that does this. It is a configurable option, "System monitor", that defaults to "xfce4-taskmanager". It can be conditionally enabled (with a checkbox; disabled by default).

When enabled, clicking anywhere on the plugin surface spawns the command and there is also a context menu item available.
When disabled, clicking does nothing and there is no menu item.

The patch introduces two new translatable labels:
"System monitor:" in the configuration dialog, and
"Run _System Monitor" in the context menu.

The widgets in the configuration dialog aren't all perfectly aligned, but that problem precedes my contribution. :D I don't write much C (or any), so you're invited to give it a thorough review. Given the existing code-base, I think I did just fine. It seems to work well. :-)

The patch was built against Debian's testing 1.1.1 archive as I didn't manage to compile the HEAD of your git repo (some autotools crazy I know nothing about).
Comment 2 Landry Breuil editbugs 2014-07-27 22:34:12 CEST
Just my 2c, before actually looking at the patch: Is there a specific reason for hardcoding 'System monitor' for the default string ? What if someone wants to run something else upon click ? Maybe it'd be better to have a more generic string, like 'command' or 'run action' ?
Comment 3 liquider 2014-07-27 22:53:27 CEST
None. But then also the right-click menu label needs editing. And the icon is set to a Gnome stock "utilities-system-monitor" [1], but I guess that's fine either way.

[1]: https://www.google.com/search?q="utilities-system-monitor"&tbm=isch

+ global->menu_item = gtk_image_menu_item_new_with_mnemonic(
+                         _("Run _System Monitor"));
+ GtkWidget* image = gtk_image_new_from_icon_name("utilities-system-monitor",
+                                                 GTK_ICON_SIZE_MENU);
+ gtk_image_menu_item_set_image(GTK_IMAGE_MENU_ITEM(global->menu_item), image);

So the commiter can change the labels to "On-click action" and "Run _custom action"? :)
Comment 4 Landry Breuil editbugs 2014-07-29 21:36:00 CEST
Ok, agreed, keeping 'system monitor' makes sense. One minor nit though with your patch (which is otherwise perfect, code-wise) - if you use say 'xterm -e top' or 'xload' as command, the panel and plugin will show the startup notification spinner until the term or xload window is closed.

Since the system monitor command will probably be spawned somewhat immediatly, i dont think we need to pass TRUE to enable startup notification here, which 'fixes' the issue.

I had a hard time understanding this comment
// HACK: this is triggered on any checkbox toggle

Until i figured out this was what showed or hid the entry in the popup menu. 

So, to avoid this, i checked the boolvar ptr against global->command.enabled's address, to make sure the currently toggled checkbox was the one enabling the command. In that case, we also dont need to call setup_monitor() anymore, so that went in an 'else' case.

All that to say i've pushed your patch (with those nits fixed) in http://git.xfce.org/panel-plugins/xfce4-systemload-plugin/commit/?id=a08eacc1afa245e4d66d29815958c67de3d6bb2f

Thanks for the patch!

Bug #3887

Reported by:
Yves-Alexis Perez
Reported on: 2008-02-28
Last modified on: 2014-07-29


CC List:
4 users


0.4.2 or older


Additional information