data: Clarify the usage of the notification ActionInvoked parameter

[3v1n0]

It's not very clear how the target value is used with the platform data, so at least avoid the potential misunderstanding, by clarifying that the target value is a variant that must be always present, even if unset

/cc @swick @jsparber

[jsparber]

Just had an in-person discussion with @swick about this. I agree the current API ActionInvoced is quite confusing.
The ActionInvoked returns the parameter as av where the first item of the array contains the variant set as target for a button so the app knows whether it has set an target for the action or not, and therefore the app also knows whether the platform data is in the first or the second position in parameter array.

We discussed also to eventually revert the changes to the ActionInvoced and introduce ActionInvoced2 that mirrors fully D-Bus Activation's ActivateAction method and would look like this:

    <signal name="ActionInvoked2">
      <arg type="s" name="app_id"/>
      <arg type="s" name="id"/>
      <arg type="s" name="action"/>
      <arg type="av" name="parameter"/>
      <arg type="a{sv}" name="platform_data"/>
    </signal>

This way the parameter only contains the button's target and the response for inline button purpose.

[jsparber]

D-Bus doesn't have the concept of empty value, so it's not possible to have an unset variant. We could set it to an empty array but that's not correct since we wouldn't be able to differentiate whether it's unset or the actual target of the button

[3v1n0]

D-Bus doesn't have the concept of empty value, so it's not possible to have an unset variant. We could set it to an empty array but that's not correct since we wouldn't be able to differentiate whether it's unset or the actual target of the button

Sure, indeed that was my main concern of this API when I saw the recent changes.

HOWEVER, indeed we can send an empty container (as for example was done in GLib IIRC), but it's not the greatest option since the API is technically new.

At the same time, since that parameter is controlled by the application, if the application, it should not care about it when the value is set to something else.

And, if we want to use an array for these values, having it of a "flexible length" it does not make sense, so or we use a fixed number of elements or we need to trash the API.

[3v1n0]

Just had an in-person discussion with @swick about this. I agree the current API ActionInvoced is quite confusing.

Yep, reason why of my (late, sorry) comments on #1298

The ActionInvoked returns the parameter as av where the first item of the array contains the variant set as target for a button so the app knows whether it has set an target for the action or not, and therefore the app also knows whether the platform data is in the first or the second position in parameter array.

That is not something we should do, having a flexible length for an array that is supposed to be in order always leave with the possibility that something in the path changed it, so I'd rather use an empty container in case, since at least we're always sure what's at each index of such array.

We discussed also to eventually revert the changes to the ActionInvoced and introduce ActionInvoced2 that mirrors fully D-Bus Activation's ActivateAction method and would look like this:

    <signal name="ActionInvoked2">
      [...]
      <arg type="a{sv}" name="platform_data"/>
    </signal>

Sure thing, and I'm glad you did since that very same API was proposed 3+ years ago in #791 since I was facing the very same issue already and I didn't want to accept workarounds, so I'm happy to reiterate there if we all agree.

[swick]

@3v1n0 You still seem to be working on related things. Mind opening a PR for reverting the original v2 changes to ActionInvoked and adding the ActionInvoked2 signal? Otherwise I will take this over (but I'd really like to not have to :) )

[3v1n0]

@3v1n0 You still seem to be working on related things. Mind opening a PR for reverting the original v2 changes to ActionInvoked and adding the ActionInvoked2 signal? Otherwise I will take this over (but I'd really like to not have to :) )

Sure, I'll prepare it

[3v1n0]

@swick btw since v2 has been technically out, should we move to v3 or wasn't it be released yet?

[swick]

I hope that no once will notice, it has been obviously broken anyway. So I would prefer to just stick to v2 and pretend this thing never happened.