[Spice-devel] [PATCH spice 14/17] spicec-x11: handle multiple types per grab

[Arnon Gilboa]

Hans de Goede wrote:
> Hi,
>
> <snip snip>
>
>>> @@ -2302,15 +2314,31 @@ static void print_targets(const char 
>>> *action, Atom *atoms, int c)
>>> static void send_targets(XEvent& request_event)
>>> {
>>> XEvent res;
>>> - /* FIXME add MULTIPLE */
>>> - /* FIXME add (and support) all 3 utf8 atom variations (see linux 
>>> agent) */
>>> - Atom targets[2] = { targets_atom, utf8_atom };
>>> + /* Warning the size of this needs to be increased each time we add 
>>> support
>>> + for a new type, or the atom count of an existing type changes */
>>> + Atom targets[4] = { targets_atom, };
>> it's a bit fishy
>
> I know, a better solution is welcome.
It's a matter of taste:), but even a "LARGE", "None"-terminated array is 
easier to maintain.
>> why not support overlap (send grab msg) of more than one atom?
>
> If there is any atom overlap (so 1 or more) for a list of atoms
> which maps to a single vdagent type (such as utf8_atoms) with the
> list of atoms reported by the owner we report the type in the
> grab message. We only need the first one which overlaps, as we
> need to pick one to send with XConvertSelection when the
> other side (the agent) requests the vdagent type in question.
>
Now I understand, but it will need to be generalized to non-utf8 types
> <snip snip>
>
>> btw, now that both client & platform have the same on_clipnboard_x() 
>> functions (which handle the opposite directions), i'm not sure it 
>> helps readability. maybe we should add _client_/_agent_ to the names?
>
> I've been thinking about that too. To me the difference is obvious, 
> one direction
> always is Platform::... the other is always clipboard_listener->... 
> but feel
> free to rename them.
It's obvious to me as well, but i guess for others it's not that easy to 
follow.
>
> Regards,
>
> Hans
Anyway,
Ack