flowDave, right, that is at least misleading if not a contradiction. But I'm not sure if my conclusion would be to get rid of the 'execute' action all together. My approach would be to make it an always possible action, in order to improve the usability of xep50.
SamWhitedhas left
guus.der.kinderenhas left
SamWhitedhas joined
dwdhas joined
guus.der.kinderenhas left
guus.der.kinderenhas left
SamWhitedhas left
SamWhitedhas joined
dwdhas left
Davehas left
Davehas left
Davehas left
guus.der.kinderenhas left
Davehas left
vanitasvitaehas left
guus.der.kinderenhas left
Davehas left
Davehas left
Davehas left
Davehas left
vanitasvitaehas joined
Davehas left
Davehas left
ralphmhas left
Davehas left
KevThe thing is, execute's entirely redundant and as-specced is broken.
KevSo while we could fix it, it would still be redundant.
dwdhas joined
Davehas left
Davehas left
guus.der.kinderenhas left
moparisthebesthas left
Davehas left
moparisthebesthas joined
flowIf you assume 'execute' to be always 'next', then yes, it is entirely redundant. If you make it a "smart" action which always points to the next likely action (which is what my PR does), then it is IMHO no longer redundant and a worthwhile feature
guus.der.kinderenhas left
KevIt's still redundant, because it's still an extra action that doesn't do anything different to the existing actions.
flowWe probably have different definitions of "redundant"
Davehas left
Davehas left
Ge0rGMaybe it's just an awkward and misleading way to tell the client which action is the default one?
dwdhas left
dwdhas joined
flowGe0rG, right now, I'm more concerned with the action, not the attribute
KevGe0rG: We have the mechanism for specifying the default already, the execute action doesn't do that.
Davehas left
dwdhas left
dwdhas joined
Davehas left
Ge0rGSo the execute attribute is the default next action, and the execute action is... some always-present default action that needs to be injected by the client?
Davehas left
dwdhas left
flowGe0rG, xep50 makes multiple statements about the execute attribute and action. Let me wikify it for your, one moment please.
Ge0rGflow: I've skimmed through the xep now, thanks
Davehas left
dwdhas joined
Davehas left
flowGe0rG, I think it comes down to the statements "The "execute" attribute specifies what the action "execute" is equivalent to." (in 4.2) and "The action "execute" is always allowed, and is equivalent to the action "next"." (in 3.4) contradicting each other.
Davehas left
flowSee also https://wiki.xmpp.org/web/XEP-Remarks/XEP-0050:Ad-Hoc_Commands
dwdhas left
Davehas left
dwdhas left
Davehas left
Davehas left
dwdhas joined
Remkohas joined
Davehas left
Davehas left
Davehas left
flowSo I've pushed a small change to my PR: https://github.com/xsf/xeps/pull/591
flowNow the only difference between Kev's PR and mine is that Kev allows "execute" to be an invalid action, whereas I specify a sane fallback so that you can simply always use "execute" as action.
dwdhas left
jonaswmaybe take that discussion to xsf@?
DaveSo I don't see the utility of an execute action given we always know what the default action presented in the UI should be.
Ge0rGmaybe take that discussion to standards@?
jonaswor even that
Ge0rGDave: I agree. Conceptually, the execute action is redundant and should best be removed.
Ge0rGHowever, I have no idea about the installed base and how it would be affected
jonaswdeprecating the use and straightening the definition would be the best course of action then?
DaveGe0rG, Sure, but deprecating it seems the action here.
dwdhas joined
Davehas left
dwdhas left
KevGe0rG: Given no-one understands the execute command at the moment, including the author, given the text, I think it's reasonable to believe the installed base is broken. Particularly as this was what caused the thread in the first place.
Davehas left
Ge0rGKev: this is just an assumption, and I'm hesitant to make changes to a Draft XEP that might break even more things.
jonaswlooks at aioxmpps Ad-Hoc implementation
jonaswit seems I happily operate on the basis of "Execute is a sane default for the action to do because it’s always allowed" and "the using protocol will know what’s needed better than the base library does".
Kevjonasw: Which is broken, because Execute isn't always allowed, because sometimes it's Next, which doesn't exist ;)
Davehas left
jonaswand the example UI I made doesn’t care about EXECUTE at all and only shows the allowed subset of cancel, next, prev and complete buttons
jonasw(sorry for caps, that’s the constant name)
KevGe0rG: There are three options: 1) Leave everything broken 2) Fix it by specifying the redundant action (possibly breaking already broken implementations) 3) Fix it by deprecating execute (possibly breaking already broken implementations)
KevI think we may as well go for the conceptually Right solution here, which is (3).
Kevjonasw: Yes, that's the sane UI. For anything UI based I can't imagine why you would ever send execute for a multi-action form.
jonaswI’m all in for (3) by the way :)
jonaswalthough, remember that the execute action is used to *start* an Ad-Hoc session :)
Ge0rGDoes (3) imply that the service always must provide at least one <action>?
Davehas left
jonaswGe0rG, since "cancel" is implied, no
jonaswbut it would make sense for a service to prefer status="completed" + an (error) message over not supplying any actions.
flowSo (3) would mean "execute" becomes a special case action that can only be used to initiate a command?
flowNote that we could possibly also do (2) and (3)
Davehas left
dwdhas joined
Zashhas left
guus.der.kinderenhas left
Kev> So (3) would mean "execute" becomes a special case action that can only be used to initiate a command?
I think it does.