ircv3 / ircv3-ideas

46 stars 3 forks source link

labelled-response: allow ACK instead of empty batch #37

Closed csmith closed 5 years ago

csmith commented 5 years ago

At the minute labeled-response requires an empty batch if a response is not otherwise generated to a command. This is quite verbose, and a little confusing to understand.

If a client labels all outgoing commands, then the usual PING/PONG exchange turns into:

server: PING 12345
client: @label=abcdefg PONG 12345
server: @label=abcdefg :irc.example.com BATCH +NMzYSq45x labeled-response
server: :irc.example.com BATCH -NMzYSq45x

And similarly PRIVMSGs when the echo-message cap isn't negotiated:

client: @label=abcdefg PRIVMSG #ircv3 :Hi
server: @label=abcdefg :irc.example.com BATCH +NMzYSq45x labeled-response
server: :irc.example.com BATCH -NMzYSq45x

Having an ACK message (or something else along those likes - such as a NOOP message) makes this a lot simplier, more readable, feels more semantic, and saves clients having to track otherwise pointless batches:

server: PING 12345
client: @label=abcdefg PONG 12345
server: @label=abcdefg :irc.example.com ACK

or:

client: @label=abcdefg PRIVMSG #ircv3 :Hi
server: @label=abcdefg :irc.example.com ACK
jesopo commented 5 years ago

huge :+1: from me on having a one-line acknowledgement - maybe :-1: for doing it this way. perhaps just a TAGMSG?

jesopo commented 5 years ago

... that said; in your first example - it would just be a response with the labeled PONG (no BATCH)

jwheare commented 5 years ago

TAGMSG is scoped to a channel or nick so wouldn't always make sense, e.g. when acking pongs.

jesopo commented 5 years ago

Fair point.

csmith commented 5 years ago

... that said; in your first example - it would just be a response with the labeled PONG (no BATCH)

That'd be if the client sent the PING, for sure. If the server sends the PING, the client sends a labeled PONG, then the server has to send something back (like a weird game of hot potato).

jesopo commented 5 years ago

also fair point

DanielOaks commented 5 years ago

we could just make an ACK message if all else fails

jwheare commented 5 years ago

Some relevant discussion from the initial issue:

https://github.com/ircv3/ircv3-specifications/pull/162#discussion-diff-34426969 @DarthGandalf "Yes. Alternatively, I can add a new verb instead:" (EMPTY)

https://github.com/ircv3/ircv3-specifications/pull/162#discussion_r34450591 @grawity "labeled-response batches seem completely pointless here (and even everywhere)." (emphasis mine)

slingamn commented 5 years ago

To clarify the discussion about PING/PONG: any message that the client labels MUST get some reply from the server, right? So clients should refrain from labeling their PONGs?

jwheare commented 5 years ago

I mean, it's pretty pointless to label stuff like PONGs, but I imagine some clients might implement this by labelling everything. Not sure it's worth explicitly banning that or restricting labels only to certain commands.

slingamn commented 5 years ago

+1 for being able to label anything, just wanted to check that my understanding that:

  1. Labeling PONG is pointless so clients should (or SHOULD) not do it
  2. There is no edge case where we actually get an infinite exchange of labeled ACKs
jwheare commented 5 years ago
  1. Yes it's pointless, but I don't think the spec should forbid or even discourage it. Would either be exhaustive (and incomplete) or short-sighted if we discouraged something that later on was deemed useful
  2. There's no edge case, since clients don't ACK ACKs. We could (SHOULD? MUST?) spec this explicitly.
SadieCat commented 5 years ago

Could we somehow generalise this by having a special batch syntax for when a spec requires a batch be used but the batch is empty? It would allow us to avoid special casing more stuff in the future.

e.g. sending * in the reference tag field of the batch command (which would have previously otherwise been +tag or -tag) like so:

C: @label=abcdefg PRIVMSG #ircv3 :Hi
S: @label=abcdefg :irc.example.com BATCH * labeled-response
jwheare commented 5 years ago

I think the point here is that it doesn't really make sense to use BATCH in this case at all. I'd rather spec something properly for a new draft feature, instead of adding something onto an existing feature.

DarthGandalf commented 5 years ago

INFO from https://github.com/ircv3/ircv3-specifications/pull/357 could be used for this?

RyanSquared commented 5 years ago

I'm thinking this is intended for the case where such responses wouldn't be sent - currently, that system is meant for extending replies and errors, so using that in a way where replies and errors don't already exist is just saying "get rid of the empty response entirely" which I'm personally fine with - I think every command could have a reasonable response, at the very least an "OK".

jesopo commented 5 years ago

I really think any of the new commands in ircv3/ircv3-specifications#357 are not designed for this - those are designed for information conveyance and this -idea is about an almost entirely thrown away message. Just seem hacky to e.g. use NOTE for this.

syzop commented 5 years ago

Personally, I don't care what is chosen. Just hope a decision/conclusion can be made. There are a few other blockers for labeled-response ratification that seem to be working toward a conclusion, so I would hate it when in the end we would have this single issue lingering and being the only blocker :)

For example, what @SaberUK mentions in https://github.com/ircv3/ircv3-ideas/issues/37#issuecomment-466210793 would be fine by me. Especially if you can come up with a case where this can be useful in other situations (other than labeled-response). But similarly, I can also understand what @jwheare says that it makes no sense to send a BATCH at all. So the ACK idea from @csmith in the opening post is also fine by me. It's also shorter.

For a client I think it's a bit unfortunate that you would have to handle receiving the labeled-response in 3 different ways, depending on: 0 reply, 1 reply and 2+ replies. Anyway, I can understand, and like I said.. I don't care much and I'm not a client dev. Since no client dev made such a comment it seems not important. For me as a server dev it's not an issue at all, I can just send whatever you want for the 0 reply case.

jesopo commented 5 years ago

From my bot dev perspective, handling a labeled response in 3 different ways is not much of a bother at all. I'd probably just store an action to be fired whenever we get a response to a command we've sent and that action can be written such as to handle the specific case.

jwheare commented 5 years ago

This is addressed in https://github.com/ircv3/ircv3-specifications/pull/386