GNOME Bugzilla – Bug 623359
Port quickopen to libpeas
Last modified: 2019-03-23 20:48:56 UTC
Created attachment 165081 [details] [review] quickopen v1 The patch attached is unfinished due to several bugs in gi and missing annotations.
Review of attachment 165081 [details] [review]: ::: plugins/quickopen/quickopen/__init__.py @@ +32,2 @@ + def do_activate(self, window): + self._window = WindowHelper(window, self) In libpeas, you should not need the window helper: the plugin *is* the window helper :-) Maybe you should merge __init__.py and windowhelper.py ::: plugins/quickopen/quickopen/popup.py @@ +25,3 @@ from virtualdirs import VirtualDirectory +class Popup(Gtk.Dialog): You miss the __gtype_name__ definition. This sucks and should be fixed in pygobject @@ +148,3 @@ + while True: + try: + info = entries.next_file(None) What if a GLib.Error happend while doing enumerate_children() ? ::: plugins/quickopen/quickopen/virtualdirs.py @@ +55,3 @@ def fill(self, screen): + #FIXME + return This should be fixed ;-) ::: plugins/quickopen/quickopen/windowhelper.py @@ +61,3 @@ def _install_menu(self): manager = self._window.get_ui_manager() + self._action_group = Gtk.ActionGroup() How do you set the action group name? Not sure it's required though.
(In reply to comment #1) > Review of attachment 165081 [details] [review]: > > ::: plugins/quickopen/quickopen/__init__.py > @@ +32,2 @@ > + def do_activate(self, window): > + self._window = WindowHelper(window, self) > > In libpeas, you should not need the window helper: the plugin *is* the window > helper :-) > > Maybe you should merge __init__.py and windowhelper.py Right. > > ::: plugins/quickopen/quickopen/popup.py > @@ +25,3 @@ > from virtualdirs import VirtualDirectory > > +class Popup(Gtk.Dialog): > > You miss the __gtype_name__ definition. This sucks and should be fixed in > pygobject > > @@ +148,3 @@ > + while True: > + try: > + info = entries.next_file(None) > > What if a GLib.Error happend while doing enumerate_children() ? Yeah I guess it should return. > > ::: plugins/quickopen/quickopen/virtualdirs.py > @@ +55,3 @@ > def fill(self, screen): > + #FIXME > + return > > This should be fixed ;-) Yeah there are several fixmes in the code, that's because it is not possible to do with gi yet, or I wasn't able. > > ::: plugins/quickopen/quickopen/windowhelper.py > @@ +61,3 @@ > def _install_menu(self): > manager = self._window.get_ui_manager() > + self._action_group = Gtk.ActionGroup() > > How do you set the action group name? Not sure it's required though. gi doesn't allow to set a name
Created attachment 165461 [details] [review] quickopen v2 Some more fixes, but still waiting for more bugs to be fixed.
Created attachment 165462 [details] [review] quickopen v3 wrong patch.
Created attachment 174978 [details] [review] patch Updated to fix a few issues with newer pygi versions etc, but still does not work
Created attachment 174998 [details] [review] patch more fixes. List is now populated and navigable, but crashes on close/select (pygobject crash?)
Created attachment 175350 [details] [review] patch
Review of attachment 175350 [details] [review]: Minor comments below ::: plugins/quickopen/quickopen/popup.py @@ +407,3 @@ self._select_index(num - 1, hasctrl, hasshift) else: + print path.get_indices() Remove debug statement ::: plugins/quickopen/quickopen/virtualdirs.py @@ +57,3 @@ items.sort(lambda a, b: cmp(b.get_visited(), a.get_visited())) + for item in items[0:self._maxitems]: This does not seem correct. It can happen that the first _maxitems items are not gedit items, and then you see nothing?
the plugin port was committed a while ago