started a port here: https://gitorious.org/d-feet/d-feet/trees/pygi-wip .

There is already a pygi branch at gnome.org.  You should have cloned from there.  I'll attach a patch I wrote at GUADEC that fixes some of the issues with the current pygi version.  I need to get my ssh-keys changes to commit them upstream.

The port at gnome.org has the service list working.  What needs to be fixed now is how we construct the tree view.

There is already a pygi branch at gnome.org.  You should have cloned from there.  I'll attach a patch I wrote at GUADEC that fixes some of the issues with the current pygi version.  I need to get my ssh-keys changes to commit them upstream.

The port at gnome.org has the service list working.  What needs to be fixed now is how we construct the tree view.

Created attachment 220300 [details] [review]
patch to make pygi  branch work with current pygi

There is already a pygi branch at gnome.org.  You should have cloned from there.  Attached is a patch I wrote at GUADEC that fixes some of the issues with the current pygi version.  I need to get my ssh-keys changes to commit them upstream.

The port at gnome.org has the service list working.  What needs to be fixed now is how we construct the tree view.

(In reply to comment #4)
> Created an attachment (id=220300) [details] [review]
> patch to make pygi  branch work with current pygi
> 
> There is already a pygi branch at gnome.org.  You should have cloned from
> there.

i tried this first but realized that d-feet doesn't even start so i started from scratch.

> The port at gnome.org has the service list working.  What needs to be fixed now
> is how we construct the tree view.

I created a new branch on gitorious based on the pygi branch from gnome.org and applied your patch [1]. i also fixed the "Connect to other bus..." dialog there [2]. I'll try to fix the treeview now.

[1] https://gitorious.org/d-feet/d-feet/commits/pygi
[2] https://gitorious.org/d-feet/d-feet/commit/815ee0f9692ad45f8bb673f5c768461b35f3f6ea

I want to clarify what we want to port to another codebase. pygtk to pygi and python-dbus to pygi gdbus already started in j5's initial patch. What about the xml stuff from dfeet/_introspect_parser.py ? imho we should use g_dbus_node_info_new_for_xml() function and then use the gdbus functions to get information about the xml. what do you think?

If g_dbus_node_info_new_for_xml is robust enough then sure, use that.  I think I use a lightweight expat parser but whatever gives a cleaner codebase is best.  Also using the g_dbus stuff will add a piece of code for people looking for an example of how to use g_dbus_node_info_new_for_xml in their own code.

The port is now ready to review. See https://gitorious.org/d-feet/d-feet .
I changed a lot of the current code. But it's less code now and I added a small and simple test suite. I'm sure there are still bugs and some features missing (see TODO). Feedback is very welcome!

I tested Thomas' branch, great work! Things that caught my eye which I think are regressions:

 - The tool bar now has three identical icons, which looks weird. I think having the three (connect to system/session/other bus) in the menu would be fine.

  Actually I think the "connect to system/session" bus should disappear entirely and these two tabs should always be there -- after all, that's the main use case of d-feet?

 - The bus name list on the left is not sorted any more. As in the original d-feet, client connections (like :1.53) should be sorted last.

 - The tree view has all items expanded, which makes it harder to find the particular method/attribute you are looking for. I think the original behaviour of starting with a collapsed tree was better.

 - I get tons of these two:

(d-feet:6550): Gtk-CRITICAL **: gtk_box_pack: assertion `gtk_widget_get_parent (child) == NULL' failed
Introspection for 'org.freedesktop.UDisks2' finished after 0.27745 s

(d-feet:6550): Gtk-CRITICAL **: gtk_container_remove: assertion `gtk_widget_get_parent (widget) == GTK_WIDGET (container) || GTK_IS_ASSISTANT (container)' failed

Otherwise this port is working really well, thanks Thomas for your great work!

I'll have a look at the diff between master and your branch as well.

I walked through the diff between master and your branch. It looks quite good, great work! I also like the code simplifications, like getting rid of the introspect_parser stuff.

I just found a couple of nitpicks:

 - Please avoid the extra spaces in the docstrings, like in

   """ show the about dialog """

 - Let's try to not introduce more py2 specific stuff like

     except Exception, e:

   (Use "Exception as e")

 - Question, why do we need this (dfeet/introspection_helper.py):

      @property
      def iface_info(self):
         return super(DBusMethod, self).iface_info

   Are properties not properly inherited? That would seem like a weird Python bug.

 - dfeet/settings.py drops the query of XDG_CONFIG_HOME and uses os.path.expanduser("~/.d-feet/config") instead. Was there a particular reason for this?

 - dfeet/_ui/wnck_utils.py does "from gi.repository import GObject as gobject" -- this could potentially clash with imported modules that try to import the "real" gobject (in fact, this is a common hack for using modules that still use the static bindings in GI programs). Could this just use GObject, to avoid confusion when reading the code?

 - d-feet.doap drops the <description>, why?

 - it completely drops HACKING. While the "johnp@" part coudl certainly do with some generalization, it is by and large still correct.

 - It drops a lot of the PNG icons and the SVG one, why?

 - Your tree has tests/test.py~ added to git, it's a backup file and should be removed.

Thanks!

(In reply to comment #9)
> I tested Thomas' branch, great work! Things that caught my eye which I think
> are regressions:
> 
>  - The tool bar now has three identical icons, which looks weird. I think
> having the three (connect to system/session/other bus) in the menu would be
> fine.

I removed the toolbar completely. Tabs for Bus Connections can be added with the menu entries. Maybe keyboard shortcuts for system/session bus connection would be nice.

>   Actually I think the "connect to system/session" bus should disappear
> entirely and these two tabs should always be there -- after all, that's the
> main use case of d-feet?

I added the system/session bus tabs per default on startup. It's still possible to remove the tabs and/or add new system/session bus tabs.

>  - The bus name list on the left is not sorted any more. As in the original
> d-feet, client connections (like :1.53) should be sorted last.

Done.

>  - The tree view has all items expanded, which makes it harder to find the
> particular method/attribute you are looking for. I think the original behaviour
> of starting with a collapsed tree was better.

Done. I also want to add a GtkSearchEntry to be able to search for interfaces/methods/signals/properties.

>  - I get tons of these two:
> 
> (d-feet:6550): Gtk-CRITICAL **: gtk_box_pack: assertion `gtk_widget_get_parent
> (child) == NULL' failed
> Introspection for 'org.freedesktop.UDisks2' finished after 0.27745 s
> 
> (d-feet:6550): Gtk-CRITICAL **: gtk_container_remove: assertion
> `gtk_widget_get_parent (widget) == GTK_WIDGET (container) || GTK_IS_ASSISTANT
> (container)' failed

Fixed. I simplified the Spinner usage in introspection.py

> Otherwise this port is working really well, thanks Thomas for your great work!
> 
> I'll have a look at the diff between master and your branch as well.

I'll try to fix the other comments. Thanks for the review, Martin!

(In reply to comment #10)
> I walked through the diff between master and your branch. It looks quite good,
> great work! I also like the code simplifications, like getting rid of the
> introspect_parser stuff.
> 
> I just found a couple of nitpicks:
> 
>  - Please avoid the extra spaces in the docstrings, like in
> 
>    """ show the about dialog """

Fixed.

>  - Let's try to not introduce more py2 specific stuff like
> 
>      except Exception, e:
> 
>    (Use "Exception as e")

Fixed.


>  - Question, why do we need this (dfeet/introspection_helper.py):
> 
>       @property
>       def iface_info(self):
>          return super(DBusMethod, self).iface_info
> 
>    Are properties not properly inherited? That would seem like a weird Python
> bug.

My fault. I experimented a bit with the properties and inheritance and committed this accidentally. Fixed now.

>  - dfeet/settings.py drops the query of XDG_CONFIG_HOME and uses
> os.path.expanduser("~/.d-feet/config") instead. Was there a particular reason
> for this?

I don't know. I started from the pygi branch. My changes are: git diff aa65bf360dc38b1006ca9e1f8d61d32bdf1ce035 -- dfeet/settings.py
Seems that J5 changed this: $ git blame dfeet/settings.py|grep expand
eabfed1b src/settings.py   (John (J5) Palmieri 2007-11-17 19:39:33 -0500 110)             self.filename = os.path.expanduser("~/.d-feet/config")

>  - dfeet/_ui/wnck_utils.py does "from gi.repository import GObject as gobject"
> -- this could potentially clash with imported modules that try to import the
> "real" gobject (in fact, this is a common hack for using modules that still use
> the static bindings in GI programs). Could this just use GObject, to avoid
> confusion when reading the code?

wnck_utils.py is not used anymore. Currently there's no application icon support in the new d-feet version. I would leave the file there so I can use it to reenable the icon support. I fixed the gobject import.

>  - d-feet.doap drops the <description>, why?

This is already changed in the original pygi branch.

>  - it completely drops HACKING. While the "johnp@" part coudl certainly do with
> some generalization, it is by and large still correct.

This is already changed in the original pygi branch.

>  - It drops a lot of the PNG icons and the SVG one, why?

This is already changed in the original pygi branch.

>  - Your tree has tests/test.py~ added to git, it's a backup file and should be
> removed.

Removed from git.

> Thanks!

Thanks!

Great work! Some more nitpicks that I stumbled upon:

 - The list of services is now ordered in reverse; it probably needs a custom ordering to be ascending, but put the ones starting with ":" last.

 - Can the list of objects on a service also be ordered, please? (Look at udisks or udisks2 on the system bus to see what I mean)

 - When expanding an object, the single "Interfaces" subtree should expand itself as well, as in the current d-feet. Same for the "Methods" and "Properties" subtrees.

 - Clicking on "Reload" gives

Traceback (most recent call last):

+ Trace 231013

Otherwise it's working really well for me. With the above fixed, I think it will be ready for John to review/merge. Thanks again!

(In reply to comment #13)
> Great work! Some more nitpicks that I stumbled upon:
> 
>  - The list of services is now ordered in reverse; it probably needs a custom
> ordering to be ascending, but put the ones starting with ":" last.

Done. I used the sort function from the current d-feet.

>  - Can the list of objects on a service also be ordered, please? (Look at
> udisks or udisks2 on the system bus to see what I mean)

Done. I added a sort function for that.

>  - When expanding an object, the single "Interfaces" subtree should expand
> itself as well, as in the current d-feet. Same for the "Methods" and
> "Properties" subtrees.

Done.

>  - Clicking on "Reload" gives

Fixed.


I also added the description to d-feet.doap.

Thanks Thomas!

Looking much nicer now. I found one regression from the latest commits, if I try to double-click on a property I get

Traceback (most recent call last):

+ Trace 231026

Single clicking on a property previously showed its value, but that doesn't work any more.

Everything else looks fine now.

(In reply to comment #15)
> Looking much nicer now. I found one regression from the latest commits, if I
> try to double-click on a property I get
> 
> Traceback (most recent call last):

Fixed. I used the wrong model for the treeview.

@John: You told me on IRC that you have not much todo with dbus theses days. So should I take maintenance for d-feet, merge the pygi branch to master and do a release when all is ready?
To be honest I have git access to git.gnome.org since a couple of weeks and I never did a release on ftp.gnome.org . So at the beginning I guess I would need some help.

Thomas, feel free to drop by in #python if you need help with doing releases. Myself, Paolo, and Ignacio all know how to do that.

I reviewed current git head. Object paths in the main window area are not sorted any more, otherwise it's working really well now. Thanks!

(In reply to comment #19)
> I reviewed current git head. Object paths in the main window area are not
> sorted any more, otherwise it's working really well now. Thanks!

Fixed. The problems was introduced with commit 77a2f06d73af968495c70e4a206f83904fb245b0 . I removed the whole GtkTreeModelSort and use the GtkTreeStore now.

Thomas, thanks for the fix for the sorting! What should I complain about now -- c'est parfait maintenant! ☺

I'm ok with Thomas taking maintainership, committing and doing a release.  Please branch the current master in a gnome2 branch.  Thanks.

I released d-feet 0.3.1 today. The release is based on the new pygi version so this this bug is solved.

Awesome, thanks Thomas!