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.

Sign up for GitHub

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

use dconf for ubuntu launcher hide/width detection #509

Merged
merged 1 commit into from
Apr 28, 2015
Merged

use dconf for ubuntu launcher hide/width detection #509

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 22 additions & 13 deletions src/guake/guake_app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,13 @@
import gobject
import gtk
import os
import platform
import subprocess
import pygtk
import sys
import xdg.Exceptions


from urllib import quote_plus
from urllib import url2pathname
from urlparse import urlsplit
Expand Down Expand Up @@ -693,21 +696,27 @@ def set_final_window_rect(self):
window_rect = screen.get_monitor_geometry(monitor)

if os.environ.get('DESKTOP_SESSION') == "ubuntu":
unity_hide = self.client.get_int(KEY('/apps/compiz-1/plugins/'
'unityshell/screen0/options/launcher_hide_mode'))
# launcher_hide_mode = 1 => autohide
if unity_hide != 1:
# Size of the icons for Unity in Ubuntu <= 12.04
# TODO Ubuntu 12.10 use dconf :
# /org/compiz/profiles/unity/plugins/unityshell/icon-size

# For Ubuntu 12.10 and above, use dconf :
# For Ubuntu 12.04 and below, use
# see if unity dock is hide => unity_hide
# and the width of unity dock. => unity_dock
if float(platform.dist()[1])>=12.10:
Copy link
Member

Choose a reason for hiding this comment

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

>>> platform.dist()
('fedora', '21', 'Twenty One')

This will go wrong in so many ways :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why? Ubuntu version always seem to be a float number.
Please give a more robust fragment of code then. Because it is not working currectly now.
both unity_hide and unity_dock get wrong value.

Copy link
Member

Choose a reason for hiding this comment

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

Well at first you would want to check if you are on ubuntu or not, because float(21) > 12.10 and that can be the case for many other OS.

But in general, I am not in favor of adding distro-specific pieces of code if we can avoid it.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, missed the check before for the DESKTOP_SESSION, I actually wonder if we would not want to replace it with a check using platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original code has if os.environ.get('DESKTOP_SESSION') == "ubuntu":, which didn't show up in diff view. these changes is under the condition that it is a ubuntu now.

The original code already a platform-specific solution. I'm just making it work. I'd like that you give a better way.

         if os.environ.get('DESKTOP_SESSION') == "ubuntu":
-            unity_hide = self.client.get_int(KEY('/apps/compiz-1/plugins/'
-                                                 'unityshell/screen0/options/launcher_hide_mode'))
-            # launcher_hide_mode = 1 => autohide
-            if unity_hide != 1:
-                # Size of the icons for Unity in Ubuntu <= 12.04
-                # TODO Ubuntu 12.10 use dconf :
-                # /org/compiz/profiles/unity/plugins/unityshell/icon-size
+
+            # For Ubuntu 12.10 and above, use dconf :
+            # For Ubuntu 12.04 and below, use
+            # see if unity dock is hide => unity_hide
+            # and the width of unity dock. => unity_dock
+            if float(platform.dist()[1])>=12.10:
+                unity_hide = int(subprocess.check_output(
+                    ['/usr/bin/dconf', 'read',
+                     '/org/compiz/profiles/unity/plugins/unityshell/launcher-hide-mode']))
+                unity_dock = int(subprocess.check_output(
+                    ['/usr/bin/dconf', 'read',
+                     '/org/compiz/profiles/unity/plugins/unityshell/icon-size']))
+            else:
+                unity_hide = self.client.get_int(KEY('/apps/compiz-1/plugins/'
+                                                     'unityshell/screen0/options/launcher_hide_mode'))
                 unity_icon_size = self.client.get_int(KEY(
                     '/apps/compiz-1/plugins/unityshell/screen0/options/icon_size'))
-                if not unity_icon_size:
-                    # If not found, it should be because of newer implementation of unity.
-                    # Dock is 64 pixel of width on my system, hope this is so on others...
-                    unity_dock = 64
-                else:
-                    unity_dock = unity_icon_size + 17
+                unity_dock = unity_icon_size + 17

Copy link
Member

Choose a reason for hiding this comment

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

indeed this block is only for ubuntu

unity_hide = int(subprocess.check_output(
['/usr/bin/dconf', 'read',
'/org/compiz/profiles/unity/plugins/unityshell/launcher-hide-mode']))
Copy link
Member

Choose a reason for hiding this comment

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

Oh please no, we do not want to shell out command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, what is the python way to do it then. I'm a python amateur.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fairly sure dconf has a python API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Then I hope that this issue is fixed the correct way soon.

Copy link
Member

Choose a reason for hiding this comment

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

i'll check that but I remember that there was some dependencies problem... so I ended up executing this command for other use case :(

Copy link
Member

Choose a reason for hiding this comment

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

arf :( we would really prefer to avoid this.

Copy link
Member

Choose a reason for hiding this comment

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

that's a chicken and egg problem. This should be handled by the packager (for debian, for rehat, ...). But, as a User, I want to be able to use Guake directly from source. And as a Maintainer, I don't want packager to introduce regression in my software.... So I don't see how to handle distribution specific stuff differently.

Maybe a good thing to do is to move this trick in a spefic module so distribution hack are better identified

unity_dock = int(subprocess.check_output(
['/usr/bin/dconf', 'read',
'/org/compiz/profiles/unity/plugins/unityshell/icon-size']))
else:
unity_hide = self.client.get_int(KEY('/apps/compiz-1/plugins/'
'unityshell/screen0/options/launcher_hide_mode'))
unity_icon_size = self.client.get_int(KEY(
'/apps/compiz-1/plugins/unityshell/screen0/options/icon_size'))
if not unity_icon_size:
# If not found, it should be because of newer implementation of unity.
# Dock is 64 pixel of width on my system, hope this is so on others...
unity_dock = 64
else:
unity_dock = unity_icon_size + 17
unity_dock = unity_icon_size + 17

# launcher_hide_mode = 1 => autohide
if unity_hide != 1:
print("correcting window width because of launcher width {} "
"(from {} to {})".format(
unity_dock, window_rect.width, window_rect.width - unity_dock))
Expand Down