[Spice-devel] [PATCH spice-gtk 2/4] Add a desktop-integration helper class

[Hans de Goede]

Hi,

On 06/22/2012 02:37 PM, Marc-André Lureau wrote:

<snip?

>> The problem then becomes defining "nothing is done there", what if we
>> are
>> compiled with dbus but not running under a supported desktop
>> environment?
>>
>> I guess I could add a warning to spice_desktop_integration_init when
>> we are
>> missing desktop integration for the users environment, should this be
>> a
>> warning, or a debug ?
>
> A warning or a debug, but not something silent.

Well, debug is silent by default, so I guess a warning?

<snip>

>>> Finally, I think the GData list isn't very appropriate structure
>>> for
>>> mapping int -> pointer, why not use a hashtable?
>>
>> Because:
>> 1) GData is meant exactly for mapping uints (quarks) to pointers, iow
>> it
>> is being used as intended (well I need mapping (unique) uints to
>> uints, but
>> mapping uints to pointers does the job nicely).
>
> quarks are "non-zero integer which uniquely identifies a particular string", not exactly uint.

And windows id's are non-zero integers which uniquely identify a particular window.

>> 2) Using a hashtable would needlessly complicate the code, since a
>> hashtable
>> is not intended for this.
>
> Many people use it to do int -> pointer (and also int -> int G(U)INT_TO_POINTER)

Just because many people use hash-tables for something does not make them the
right choice, I looked at using hash-tables first, before I choose to use
GData and I found GData to be a better match.

>>> My understanding of this logic could also be made with a refcount.
>> No we need to cookie associated to the specific toplevel window id to
>> uninhibit.
>
> ah right, /blame me, I didn't spend enough time reviewing the code..
>
> Could we simplify the API and the logic if we used a hidden window? have you thought about it? That way, we don't need to pass a window id around, we don't need to call (un)inhibit many times, we don't need to maintain int->int mapping, we only keep one cookie and a refcount.

Erm, no, the whole purpose of the toplevel window id is so that if the window goes
away gnome-session can automatically uninhibit if the app does not do so properly,
keeping a hidden window around for this would break this.

Regards,

Hans