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

Added StatusNotifier backend for KDE 4 (and 5) #96

Merged
merged 3 commits into from
Feb 25, 2015
Merged

Added StatusNotifier backend for KDE 4 (and 5) #96

merged 3 commits into from
Feb 25, 2015

Conversation

ntninja
Copy link

@ntninja ntninja commented Feb 22, 2015

Fix for #73

This code has been tested in several desktop environments (see the "compatibility table" in syncthing_gtk/statusicon.py).
I tried to minimize the amount of the desktop probing as far as possible, but some checks remain. By default the gtk.StatusIcon()-based back-end will be loaded and will then check whether GTK succeeded in embedding the icon (.is_embedded()). If it didn't work, the other backends are tried in order (AppIndicator, KDE4, Dummy on Unity; KDE4, AppIndicator, Dummy on non-Unity) until one of them initializes correctly. Note that the GTK back-end will always fail on Unity and KDE.
There also used to also be a Qt5-based back-end but it was very unstable and required several hacks to prevent it from crashing the application. (Somehow, after calling PyQt5.Qt.QApplication(), every attempt by GTK/Cairo/Pango to query Xrdb would cause a crash inside of Xlib...) As I couldn't find a convincing way to prevent the crashes (one that doesn't look like it's going to break tomorrow) I decided to ditch it entirely (historical reference).

Other things:

  • The menu item Show/Hide window is now always visible. It could be hidden on back-ends that support right-clicking but it wouldn't be worth it IMHO... (If you really insist on it I'll try adding that of course)
  • The main window isn't forcibly shown anymore when it's clear that no icon is shown. I could add a signal that is dispatched whenever the Dummy back-end is loaded, but just because a back-end loads correctly it DOES NOT mean that an icon will be visible so that would be pretty unreliable at best. (Yes, I'd add that as well if you insist ;-) )

@kozec
Copy link
Owner

kozec commented Feb 23, 2015

Looks good, I'll do some testing as well. Only two things:

The main window isn't forcibly shown anymore when it's clear that no icon is shown. I could add a signal that is dispatched whenever the Dummy back-end is loaded

Showing window is certainly better than doing nothing visible, even if chances of being ran on DE that doesn't support any notifications at all areprobably slim.

And second, I'm slightly worried about

from gi.repository import GObject as gobject
from gi.repository import GLib    as glib
from gi.repository import Gtk     as gtk

Rest of code imports G* stuff capitalized and having one file that does it in other way would break at least code completion, if nothing else ;)

Changed gtk.*, glib.* and gobject.* references in `statusicon.py` back to their original name (by request)

Added some extra documentation in `statusicon.py`

Made the destinction in `statusicon.py` clearer between private, protected and public object members
@ntninja
Copy link
Author

ntninja commented Feb 24, 2015

Updated and tested :-)

self.set_property("active", is_embedded)

def _on_rclick(self, si, button, time):
self._popupmenu.popup(None, None, None, None, button, time)
Copy link
Owner

Choose a reason for hiding this comment

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

Small issue from testing:

__popupmenu (with two underscores) is used everywhere else. Thanks to that, I'm getting

Traceback (most recent call last):
  File "c:\syncthing-gui\syncthing_gtk\statusicon.py", line 246, in _on_rclick
    self._popupmenu.popup(None, None, None, None, button, time)
AttributeError: 'StatusIconGTK3' object has no attribute '_popupmenu'

;-)

@ntninja
Copy link
Author

ntninja commented Feb 24, 2015

Sorry for that! I didn't test the updated commit as throughly as the original one, so I guess some things haven't been tested well (I only tested left-click, but not right-click with the GTK3 back-end for instance ;-) )

@kozec
Copy link
Owner

kozec commented Feb 25, 2015

Ok, I did some testing as well and it looks like everything works as expected. Thank you for your help :)

kozec added a commit that referenced this pull request Feb 25, 2015
Added StatusNotifier backend for KDE 4 (and 5)
@kozec kozec merged commit da715b5 into kozec:master Feb 25, 2015
@ntninja
Copy link
Author

ntninja commented Feb 25, 2015

Thank you :-)

Repository owner locked and limited conversation to collaborators Nov 9, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants