Re: [GNUnet-developers] Various gnunet-gtk issues

[Christian Grothoff]

On Monday 09 June 2008 11:56:09 am Milan Bouchet-Valat wrote:
> Le lundi 09 juin 2008 à 10:10 -0600, Christian Grothoff a écrit :
> > On Monday 09 June 2008 05:52:43 am Milan Bouchet-Valat wrote:
> > > Hi all!
> > >
> > > I changed once again the layout of the treeview for search results:
> > > - now it looks like Nautilus and the GtkFileChooser: we gain in
> > > consistency, eye-candy, and a little space is freed; the hierarchy
> > > directory/children is much more clear now
> > > - status logos are nicer and free a lot of space, allowing to see the
> > > meta-data
> >
> > If they are understood -- maybe we should add tooltips here?
>
> Sure (see below).
>
> > > Question: was does GNUNET_URITRACK_DIRECTORY_ADDED means? I've chosen a
> > > '+' icon (GTK_STOCK_ADD) for this status, but I did not really
> > > understand it...
> >
> > It means that the file was published as part of a directory.  Using the
> > same icon (Add) as for normal publications is probably fine.
>
> I'll do this very soon (one line).
>
> > Also,
> >
> > http://library.gnome.org/devel/gtk/2.11/GtkIconTheme.html#gtk-icon-theme-
> >load-icon
> >
> > says that we need to unref the icon after use.  Since you do not do that
> > (and since you're re-loading them each time they are needed) the
> > reference counts will go crazy on those.  I think we should add a local
> > cache for those pixbufs.
>
> That was one of my concerns. AFAIK, the GtkTreeStore calls g_object_ref()
> when we add an entry with this pixbuf, and calls g_object_unref() when it
> is changed/destroyed...
> >From the adress above:
>
> "Returns : the rendered icon; this may be a newly created icon or a new
> reference to an internal icon, so you must not modify the icon. Use
> g_object_unref() to release your reference to the icon."
> As I understand it, the pixbuf is only loaded once. When we close a
> search the model is destroyed and the ref counts are released. Isn't
> that good?

The problem is that adding the pixbuf to the tree model does another ref 
(2-1=1, so the object won't get destroyed...).

> > > Now there's a problem: we cannot sort files by type. The previous
> > > 'Type' column with icons was not good either, because icons regroup
> > > many different MIME types, which would have been separated without
> > > explanation when sorting.
> >
> > I didn't see this as a problem.  Not being able to sort by mime type at
> > all, that's more of a problem IMO.
>
> Well the problem was the files would have been sorted, but you would not
> have known what was their type...

Well, the file extension or the mime-type in the meta-data info dialog (right 
click...) would have most likely given it away.

> > > I wonder whether we should add a column with
> > > the type description after the meta-data one: icons don't tell what
> > > precise format is the file (could be solved with a tooltip, though),
> > > and putting it before meta-data would partially hide them. This would
> > > allow sorting by type when required (no so common case, maybe). What do
> > > you think?
> >
> > I think a tooltip would be nice (instead of wasting space with the mime
> > text). However, I do know that tooltips are notoriously awkward with
> > GtkTreeViews (may have improved in recent GTK+ versions, but I don't
> > think we can rely on having those).  So good luck implementing them...
>
> I'll have a look at this. I think latest GTK did improve this greatly.
> More #ifdefs in perspective... Do you want this for 0.8.1? I guess you'd
> prefer I slow down coding for 0.8.0.

You don't need to slow down -- just before you commit test things thoroughly 
so that you don't introduce new bugs...

> > > Apart from that, I've been thinking for a while of removing the 'Search
> > > Overview' list from 'Activity' so that more space is available for,
> > > esp., downloads. The overview is already available with the tabs in
> > > 'Search and download', anyway. With this change, 'Search and download'
> > > could be set as the first tab, thus the default in FS (since it is the
> > > first step and most useful one). Comments?
> >
> > Makes sense to me.
>
> Same question: as it is easy, should I try to get this in 0.8.0, or
> later? Not a problem for me.

Same answer: if you can be sure not to introduce new bugs, polishing is ok.

> > > Also, I've noticed a strange behavior of the 'Stop' download button.
> > > Contrary to the 'Delete' one, it does not update the search views, and
> > > the "cancelled" status is not set. This is quite inconsistent.
> >
> > I think I've fixed this, but I cannot test it (see comment on your r7104
> > below).
>
> I still see it. I'll see what I can do.
>
> > > And last but not least: when do you plan to release 0.8? It could be
> > > nice for me to avoid committing buggy code just when you try to
> > > stabilize all this stuff. This will be a great release!
> >
> > The answer has already been posted: https://gnunet.org/drupal/node/320
> > So yes, it is time to fix stuff, not break stuff ;-).
>
> Sorry, I read Drupal very often, but since I was coding... ;-) You could
> just have sent a mail to the list too, this would have made me calm down
> quite efficiently.

Well, the goal was more to give people like you more energy, not less ;-).

> > Also, your r7104 breaks stuff -- you cannot just rename handlers like
> > that, they are referenced in the glade file  (and now clear/stop no
> > longer work!!!). Looks like this may have been an incomplete commit --
> > please fix...
>
> You made me realize that most of my code was not committed, which explains
> this great inconsistency. What a shame! I'll never use GUIs to SVN again! I
> hope you will like more revision 7111, and sorry for that mistake (once
> again).
>
> Anyway just tell me if there are other bugs that I introduced, I'll try
> to fix them ASAP.

Will do.  You might also want to join us on IRC...

Christian