Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
csmith opened this issue Feb 21, 2019 · 20 comments
Closed

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

csmith opened this issue Feb 21, 2019 · 20 comments

Comments

@csmith
Copy link

csmith commented Feb 21, 2019

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
Copy link

jesopo commented Feb 21, 2019

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

@jesopo
Copy link

jesopo commented Feb 21, 2019

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

@jwheare
Copy link
Member

jwheare commented Feb 21, 2019

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

@jesopo
Copy link

jesopo commented Feb 21, 2019

Fair point.

@csmith
Copy link
Author

csmith commented Feb 21, 2019

... 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
Copy link

jesopo commented Feb 21, 2019

also fair point

@DanielOaks
Copy link
Member

we could just make an ACK message if all else fails

@jwheare
Copy link
Member

jwheare commented Feb 21, 2019

Some relevant discussion from the initial issue:

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

ircv3/ircv3-specifications#162 (comment)
@grawity "labeled-response batches seem completely pointless here (and even everywhere)." (emphasis mine)

@slingamn
Copy link

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
Copy link
Member

jwheare commented Feb 21, 2019

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
Copy link

+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
Copy link
Member

jwheare commented Feb 21, 2019

  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
Copy link

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
Copy link
Member

jwheare commented Feb 22, 2019

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
Copy link
Member

INFO from ircv3/ircv3-specifications#357 could be used for this?

@RyanSquared
Copy link

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
Copy link

jesopo commented Feb 24, 2019

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
Copy link

syzop commented May 11, 2019

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 #37 (comment) 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
Copy link

jesopo commented May 11, 2019

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
Copy link
Member

jwheare commented May 29, 2019

This is addressed in ircv3/ircv3-specifications#386

@jwheare jwheare closed this as completed May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants