[Xfce4-commits] r24827 - libxfcegui4/branches/xfce_4_4/libxfcegui4

[Danny Milosavljevic]

Hi,

On Sat, 03 Feb 2007 21:55:39 -0800, Brian J. Tarricone wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: RIPEMD160
> 
> Danny Milosavljevic wrote:
>> Author: dannym
>> Date: 2007-02-03 14:19:40 +0000 (Sat, 03 Feb 2007)
>> New Revision: 24827
>> 
>> Modified:
>>    libxfcegui4/branches/xfce_4_4/libxfcegui4/netk-window.c
>>    libxfcegui4/branches/xfce_4_4/libxfcegui4/netk-xutils.c
>>    libxfcegui4/branches/xfce_4_4/libxfcegui4/netk-xutils.h
>> Log:
>> add netk_window_get_icon_geometry
> 
> Uh, Danny, why are you adding API to the 4.4 branch?  Please don't.  

I'm tying up loose ends.
In this case there was a "netk_window_set_icon_geometry" but no "netk_window_get_icon_geometry".

> If there's a valid reason for this, you should be discussing it on the ML, or at the very least providing a rationale in your commit message.

Ok then:
I added "netk_window_get_icon_geometry" because it wasn't there and "netk_window_set_icon_geometry" was. 
It's reviewed and should be safe to use.

(I'm not changing how existing functionality works)

I insist on this NetkWindow change because there is no other way to read the icon geometry (safe from repeating the same Xlib code in every application).

> Ditto for the IMHO useless convenience functions you've added to
> NetkTrayIcon.  There's no reason to clutter the exports table with
> one-line functions.

These 2 functions just make set_screen/new symmetric to gtk_window_get_screen.

(In this case, changing "netk_tray_icon_new" would have been nicer, but it would break API/ABI. Therefore, I added two new ones.)

I don't see how 2 functions "clutter" the exports table and I was not aware that we had an objective to "not clutter the exports table". I am not in a programming contest trying to get the export table down to 2 entries or something. 

Anyway, I don't insist on these NetkTrayIcon changes, they are merely cosmetic.

> Why mess with NetkTasklist?  It's old and crusty and will be rewritten  sometime in the 4.5 cycle.

Because there were setters and no getters and the structure is private.
I.e. to fix a bug in 4.4.0 (being able to set something but not being able to read it is a bug, especially - but not limited to - when the structure is private).

Just because it will be rewritten sometime in the 4.5 cycle doesn't mean we shouldn't fix obvious bugs in the 4.4 maintainance releases.

I insist on these NetkTasklist changes because there is no other way to get the properties "grouping", "switch_workspace_on_unminimize", "grouping_limit", "include_all_workspaces", "show_label", "button_relief".

Or do you suggest I use pointer arithmetic with the instance pointer to get them?

> At any rate, NONE of this stuff should be added to the 4.4 branch.

Every change I did except the ones on NetkTrayIcon was neccessary to fix API flaws and I don't see why I should not have done them.

cheers,
  Danny