Merge lp:~pitti/gtimelog/pygi into lp:~gtimelog-dev/gtimelog/trunk
- pygi
- Merge into trunk
Status: | Merged |
---|---|
Merge reported by: | Marius Gedminas |
Merged at revision: | not available |
Proposed branch: | lp:~pitti/gtimelog/pygi |
Merge into: | lp:~gtimelog-dev/gtimelog/trunk |
Diff against target: |
203 lines (+72/-24) 3 files modified
HACKING.txt (+2/-1) NEWS.txt (+10/-0) src/gtimelog/main.py (+60/-23) |
To merge this branch: | bzr merge lp:~pitti/gtimelog/pygi |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Martin Pitt (community) | Needs Resubmitting | ||
Marius Gedminas | Needs Fixing | ||
Review via email: mp+47696@code.launchpad.net |
Commit message
Description of the change
This ports gtimelog from pygtk to gobject-
This mostly works fine, but the only thing that I can't get to work for the life of it is the detection whether to use the dark or bright icon. I played with get_style_
Do you know whether there is a cleaner way to ship icons for dark and bright themes?
Marius Gedminas (mgedmin) wrote : | # |
Barry Warsaw (barry) wrote : | # |
BTW, note that the dark background icon has *never* worked for me. For example, I'll attach a screenshot from Natty with stock gtimelog.
Barry Warsaw (barry) wrote : | # |
Er, okay, I forgot you can't add attachments to merge proposals. :(
Martin Pitt (pitti) wrote : | # |
> But something seems to be wrong: the diff is empty.
Argh, sorry. Pushed.
> Is it possible to have one codebase that uses either PyGtk or PyGI?
That's a bit tricky in general. I can change the code to work with either, but that will inevitably result in a few more "if using pygi; then ... else pygtk way" constructs.
Martin Pitt (pitti) wrote : | # |
I updated the branch now to work with PyGTK, GTK2-GI, and GTK3-GI. For using GTK3, comment out "gtk.require_
BTW, the pygtk "get theme color" hack doesn't seem to work under unity either.
So while the code looks a bit ugly now in some places due to the four "if pygtk:" checks, it's by and large still ok, I think. The pygtk bits can be dropped in a year or so when GTK3 and GI become the ubiquitous standard.
- 177. By Marius Gedminas
-
Bugfix: always preserve the order of entries, even when they have the same
timestamp (LP: #708825). - 178. By Marius Gedminas
-
Application Indicator menu now has a CheckMenuItem "Show GTimelog".
Previously it used the old tray icon menu with separate Show/Hide items.
- 179. By Marius Gedminas
-
Prepare to release 0.5.0
- 180. By Marius Gedminas
-
Post-release version bump
Marius Gedminas (mgedmin) wrote : | # |
Looks good to me.
At the moment I'm having intermittent connection problems (connection refused on port 22) that prevent me from merging your branch, so I'll do that a bit later. I barely managed to push out a bugfix (unrelated to GUI code) and a small change to the app indicator menu earlier today. Speaking of which, I hope working GtkCheckMenuItems hasn't changed in any significant way between PyGtk and PyGI?
Also, we're hosting a 48-hour Game Jam in our office, so I might be a bit busy.
I think I'd also like try a few more refactorings, e.g. those constructor calls can also be handled the same way as constants:
try:
# PyGI
...
...
except ImportError:
# PyGTK
...
...
...
tabs = new_pango_
...
and constants such as PANGO_TAB_LEFT would look a bit more conventional in upper case. But those are minor details.
Martin Pitt (pitti) wrote : | # |
Good idea, I made those two changes.
Marius Gedminas (mgedmin) wrote : | # |
Unfortunately the code doesn't work on Maverick:
Traceback (most recent call last):
File "./gtimelog", line 11, in <module>
from gtimelog.main import main
File "./src/
gtk.
File "/usr/lib/
attribute = getattr(
File "/usr/lib/
self.
AttributeError: 'DynamicModule' object has no attribute 'require_version'
Some kind of feature/version check may be necessary. Or ir may be enough to swap the try/except parts and try PyGTK by default, falling back to PyGI when PyGTK is not available.
Also, could you add "Contributed by Martin Pitt <email address hidden>" to the changelog entry?
Barry Warsaw (barry) wrote : | # |
Minor comments about the code:
the 'pygtk = True' in the ImportError clause looks odd because you're clobbering the name binding given by the 'import pygtk'. It would probably read easier if you changed that to 'has_pygtk = True'.
OTOH, the way it's often done in other places is to set 'pygtk = None' in the try: clause. You're 'if pygtk' tests will still work that way because the import statement will bind pygtk to the module object, which is not false.
Can you tighten up the bare except on line 130? If not, *please* add a comment explaining why the bare except is necessary.
Other than that, it's encouraging that it wasn't really that much code to add support for the three different regimes. Very nice!
Martin Pitt (pitti) wrote : | # |
Ah, if we get an AttributeError after the "Gtk" repository is already imported, we can't import pygtk afterwards, so due to that we actually have to test for pygtk first. That will cause pygtk to be preferred, and the GI code to get much less exposure, but we can still locally patch that in Ubuntu, so it's not a biggie.
I added the bare "except:" on the indicator because ImportError is not the only probable exception here. gi.RepositoryError also can happen if the installed AppIndicator gir wasn't built against the version of GTK that gtimelog uses. There might be other causes I haven't thought about as well, but I guess those two are the main ones, so I tightened up the except statement.
I addressed the other things as well.
Marius Gedminas (mgedmin) wrote : | # |
Thanks, merged!
- 181. By Marius Gedminas
-
Support PyGI/PyGObject when PyGtk is not available.
Preview Diff
1 | === modified file 'HACKING.txt' |
2 | --- HACKING.txt 2011-01-08 14:38:47 +0000 |
3 | +++ HACKING.txt 2011-01-29 10:43:14 +0000 |
4 | @@ -26,7 +26,8 @@ |
5 | Here's how you can create and send me a a patch. You'll need the `Bazaar |
6 | <http://bazaar.canonical.com/en/>`_ version control system installed on your |
7 | computer, as well as the usual stuff needed to run GTimeLog (`Python |
8 | -<http://python.org/>`_, `PyGtk <http://pygtk.org/>`_). |
9 | +<http://python.org/>`_, and `PyGtk <http://pygtk.org/>`_ or the GTK and Pango |
10 | +gobject-introspection typelibs). |
11 | |
12 | First, get a copy of the source code:: |
13 | |
14 | |
15 | === modified file 'NEWS.txt' |
16 | --- NEWS.txt 2011-01-28 13:58:01 +0000 |
17 | +++ NEWS.txt 2011-01-29 10:43:14 +0000 |
18 | @@ -12,6 +12,16 @@ |
19 | src/gtimelog/gtimelog.ui. It needs to be installed into |
20 | /usr/share/gtimelog/. |
21 | |
22 | +* Ported from PyGTK to GI. This supports GTK 2 and GTK 3 with GI now, but still |
23 | + works with PyGTK. |
24 | + |
25 | + Packager's note: If you want to use GI, you need to change the package's |
26 | + dependencies from pygtk to the package that provides the GTK and Pango |
27 | + typelibs (e. g. gir1.2-gtk-2.0 and gir1.2-pango-1.0 on Debian/Ubuntu). It |
28 | + also requires pygobject >= 2.27.1. |
29 | + |
30 | + Contributed by Martin Pitt <martin.pitt@ubuntu.com>. |
31 | + |
32 | * GTimeLog now supports Ubuntu's application indicators. There's a new |
33 | configuration option, ``prefer_app_indicator``, defaulting to true. |
34 | Fixes LP: #523461. |
35 | |
36 | === modified file 'src/gtimelog/main.py' |
37 | --- src/gtimelog/main.py 2011-01-28 13:53:34 +0000 |
38 | +++ src/gtimelog/main.py 2011-01-29 10:43:14 +0000 |
39 | @@ -15,11 +15,34 @@ |
40 | import ConfigParser |
41 | from operator import itemgetter |
42 | |
43 | -import pygtk |
44 | -pygtk.require('2.0') |
45 | import gobject |
46 | -import gtk |
47 | -import pango |
48 | + |
49 | +# we have to try pygtk first, then fall back to GI; if we have a too old GI |
50 | +# (without require_version()), we can't import pygtk on top of gi.repo.Gtk. |
51 | +try: |
52 | + import pygtk |
53 | + pygtk.require('2.0') |
54 | + import gtk |
55 | + from gtk import gdk as gdk |
56 | + import pango |
57 | + |
58 | + PANGO_ALIGN_LEFT = pango.TAB_LEFT |
59 | + GTK_RESPONSE_OK = gtk.RESPONSE_OK |
60 | + gtk_status_icon_new = gtk.status_icon_new_from_file |
61 | + pango_tabarray_new = pango.TabArray |
62 | +except ImportError: |
63 | + import gi |
64 | + from gi.repository import Gdk as gdk |
65 | + from gi.repository import Gtk as gtk |
66 | + gtk.require_version('2.0') |
67 | + from gi.repository import Pango as pango |
68 | + pygtk = None |
69 | + |
70 | + # these are hacks until we fully switch to GI |
71 | + PANGO_ALIGN_LEFT = pango.TabAlign.LEFT |
72 | + GTK_RESPONSE_OK = gtk.ResponseType.OK |
73 | + gtk_status_icon_new = gtk.StatusIcon.new_from_file |
74 | + pango_tabarray_new = pango.TabArray.new |
75 | |
76 | try: |
77 | import dbus |
78 | @@ -995,6 +1018,10 @@ |
79 | # XXX assumes the panel's color matches a menu bar's color, which is |
80 | # not necessarily the case! this logic works for, say, |
81 | # Ambiance/Radiance, but it gets New Wave and Dark Room wrong. |
82 | + if not pygtk: |
83 | + # XXX: need to figure out how to do that with gi |
84 | + return icon_file_bright |
85 | + |
86 | menu_text_color = gtk.MenuBar().rc_get_style().text[gtk.STATE_NORMAL] |
87 | if menu_text_color.value >= 0.5: |
88 | return icon_file_bright |
89 | @@ -1012,7 +1039,7 @@ |
90 | if not hasattr(gtk, 'StatusIcon'): |
91 | # you must be using a very old PyGtk |
92 | return |
93 | - self.icon = gtk.status_icon_new_from_file(self.icon_name) |
94 | + self.icon = gtk_status_icon_new(self.icon_name) |
95 | self.last_tick = False |
96 | self.tick() |
97 | self.icon.connect("activate", self.on_activate) |
98 | @@ -1049,7 +1076,7 @@ |
99 | |
100 | def tick(self): |
101 | """Tick every second.""" |
102 | - self.icon.set_tooltip(self.tip()) |
103 | + self.icon.set_tooltip_text(self.tip()) |
104 | return True |
105 | |
106 | def tip(self): |
107 | @@ -1075,14 +1102,24 @@ |
108 | self.gtimelog_window = gtimelog_window |
109 | self.timelog = gtimelog_window.timelog |
110 | self.indicator = None |
111 | - try: |
112 | - import appindicator |
113 | - except ImportError: |
114 | - return # nothing to do here, move along |
115 | + if pygtk: |
116 | + try: |
117 | + import appindicator |
118 | + self.indicator = appindicator.Indicator("gtimelog", self.icon_name, |
119 | + appindicator.CATEGORY_APPLICATION_STATUS) |
120 | + self.indicator.set_status(appindicator.STATUS_ACTIVE) |
121 | + except ImportError: |
122 | + return # nothing to do here, move along |
123 | # or install python-appindicator |
124 | - self.indicator = appindicator.Indicator("gtimelog", self.icon_name, |
125 | - appindicator.CATEGORY_APPLICATION_STATUS) |
126 | - self.indicator.set_status(appindicator.STATUS_ACTIVE) |
127 | + else: |
128 | + try: |
129 | + from gi.repository import AppIndicator |
130 | + self.indicator = AppIndicator.Indicator.new("gtimelog", self.icon_name, |
131 | + AppIndicator.IndicatorCategory.APPLICATION_STATUS) |
132 | + self.indicator.set_status(AppIndicator.IndicatorStatus.ACTIVE) |
133 | + except (ImportError, gi._gi.RepositoryError): |
134 | + return |
135 | + |
136 | self.indicator.set_menu(gtimelog_window.app_indicator_menu) |
137 | self.gtimelog_window.tray_icon = self |
138 | self.gtimelog_window.main_window.connect("style-set", self.on_style_set) |
139 | @@ -1304,9 +1341,9 @@ |
140 | """Set up tab stops in the log view.""" |
141 | pango_context = self.log_view.get_pango_context() |
142 | em = pango_context.get_font_description().get_size() |
143 | - tabs = pango.TabArray(2, False) |
144 | - tabs.set_tab(0, pango.TAB_LEFT, 9 * em) |
145 | - tabs.set_tab(1, pango.TAB_LEFT, 12 * em) |
146 | + tabs = pango_tabarray_new(2, False) |
147 | + tabs.set_tab(0, PANGO_ALIGN_LEFT, 9 * em) |
148 | + tabs.set_tab(1, PANGO_ALIGN_LEFT, 12 * em) |
149 | self.log_view.set_tabs(tabs) |
150 | |
151 | def w(self, text, tag=None): |
152 | @@ -1442,7 +1479,7 @@ |
153 | def scroll_to_end(self): |
154 | buffer = self.log_view.get_buffer() |
155 | end_mark = buffer.create_mark('end', buffer.get_end_iter()) |
156 | - self.log_view.scroll_to_mark(end_mark, 0) |
157 | + self.log_view.scroll_to_mark(end_mark, 0, False, 0, 0) |
158 | buffer.delete_mark(end_mark) |
159 | |
160 | def set_up_task_list(self): |
161 | @@ -1593,7 +1630,7 @@ |
162 | |
163 | Returns either a datetime.date, or one. |
164 | """ |
165 | - if self.calendar_dialog.run() == gtk.RESPONSE_OK: |
166 | + if self.calendar_dialog.run() == GTK_RESPONSE_OK: |
167 | y, m1, d = self.calendar.get_date() |
168 | day = datetime.date(y, m1+1, d) |
169 | else: |
170 | @@ -1603,7 +1640,7 @@ |
171 | |
172 | def on_calendar_day_selected_double_click(self, widget): |
173 | """Double-click on a calendar day: close the dialog.""" |
174 | - self.calendar_dialog.response(gtk.RESPONSE_OK) |
175 | + self.calendar_dialog.response(GTK_RESPONSE_OK) |
176 | |
177 | def weekly_window(self, day=None): |
178 | if not day: |
179 | @@ -1808,20 +1845,20 @@ |
180 | |
181 | def task_entry_key_press(self, widget, event): |
182 | """Handle key presses in task entry.""" |
183 | - if event.keyval == gtk.gdk.keyval_from_name('Prior'): |
184 | + if event.keyval == gdk.keyval_from_name('Prior'): |
185 | self._do_history(1) |
186 | return True |
187 | - if event.keyval == gtk.gdk.keyval_from_name('Next'): |
188 | + if event.keyval == gdk.keyval_from_name('Next'): |
189 | self._do_history(-1) |
190 | return True |
191 | # XXX This interferes with the completion box. How do I determine |
192 | # whether the completion box is visible or not? |
193 | if self.have_completion: |
194 | return False |
195 | - if event.keyval == gtk.gdk.keyval_from_name('Up'): |
196 | + if event.keyval == gdk.keyval_from_name('Up'): |
197 | self._do_history(1) |
198 | return True |
199 | - if event.keyval == gtk.gdk.keyval_from_name('Down'): |
200 | + if event.keyval == gdk.keyval_from_name('Down'): |
201 | self._do_history(-1) |
202 | return True |
203 | return False |
Awesome!
But something seems to be wrong: the diff is empty. Have you pushed your changes to the launchpad branch?
Is it possible to have one codebase that uses either PyGtk or PyGI? I'd like GTimeLog to continue working with older distributions like Lucid.
The icon color detection is an ugly hack. I think the proper solution is to install different versions of gtimelog.png into /usr/share/ icons/ubutu- mono-{dark, light}, and perhaps a colourful one into /usr/share/ icons/hicolor.
The hack is there mostly because development is easier if I can run gtimelog (and see its icon) directly from a source checkout.