Re: Fixing window grouping



On Tue, 2003-09-30 at 00:22, Federico Mena Quintero wrote:
> - The title of the group buttons comes from the WM_CLASS.res_class. 
> This is an ugly string --- we could probably get something better from
> the corresponding WnckApplication object for those windows that have it.
> 
> - Icons for groups don't work.  We can probably do the same as the
> preceding item.

Agree with these, I would essentially get your name something like:

 - pick name of an arbitrary WnckApplication in the class group
 - if no name set on any WnckApplication, if all the windows in the 
     group have the same title, you could use that
 - otherwise use the name of the class

Or other heuristic along these lines. Of course icon works the same way.
As with WnckApplication the result should be cached, otherwise next
thing you know some app contains something like:

 for (tmp = list; tmp != NULL; tmp = tmp->next)
   {
       if (strcmp (name, wnck_class_group_get_name (group)) == 0)
          frobate();
   }

and there's a round trip on every iteration.

Other comments:
 
 - if adding/removing/changing ABI, the libtool versioning needs to be
     updated in configure.in

 - I don't really understand what's going on in tasklist.c, 
     specifically why WNCK_TASK_APPLICATION continues to exist

 - there are various FIXME and #if 0
 
 - WnckClassGroup has a get_name() method returning the class, 
     I would have: get_name() (human-readable name), get_class or 
     get_res_class returning the actual res class string, and
get_icon().

 - in screen.h you break the ABI, which means the libtool needs 
     adjusting as above, but also it's time to add padding to all the 
     vtables (see WnckScreenClass #if 0)

 - _wnck_get_res_class_utf8() is a round trip; as such, I think it 
     should be done only once. Which is just a matter of using 
     wnck_window_get_resource_class instead in screen.c I think.

 -  You are messing up my beautiful while loops! for loops are the 
      spawn of evil!

 - shouldn't your copyright notice say Novell? ;-)  

Thanks for the patch; does this mean you're the libwnck maintainer now?

Havoc




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]