Update metadata spec#613
Conversation
The errata section at the bottom lists all normative changes. Additionally, the specification was rearranged and copyedited to flow more nicely when read from top-to-bottom. It should (hopefully) be possible to now follow the specification without needing to jump around, and all concepts are (hopefully) introduced before being used in the specification. Some examples were moved inline to help illustrate, although the rather large examples section at the end was maintained in case it was helpful.
|
|
||
| * A previous version of this specification sent `RPL_KEYVALUE` as the response to `METADATA CLEAR`. This was changed to `RPL_KEYNOTSET` to simplify client processsing. | ||
| * A previous version of this specification sent `761 RPL_KEYVALUE` as the response to `METADATA CLEAR`. This was changed to `766 RPL_KEYNOTSET` to simplify client processsing. | ||
| * The `METADATA` server message was removed; `761 RPL_KEYVALUE` and `766 RPL_KEYNOTSET` are used in all cases to advertise key values or the lack thereof. |
There was a problem hiding this comment.
This breaks the longstanding norm (albeit violated by monitor) that numerics are for replies and verbs are for asynchronous messages. I don't see a strong reason to uphold the norm at this time, but I wanted to flag it in case anyone else has opinions (I think I remember @syzop commenting on this in the past?)
There was a problem hiding this comment.
extended-isupport also breaks this norm. Largely, I see no reason why we need two parallel notification mechanisms. Either the spec should be 100% RPL_KEYVALUE/RPL_KEYNOTSET or 100% METADATA notifications. I chose the numerics because I see value in separating "this key has a value" vs "this key does not exist" and the previous specification wording of METADATA notification didn't cover the "does not exist" case at all.
There was a problem hiding this comment.
Ergo sends METADATA with no final parameter when a key is unset (although this was not explicit in the specification).
That's a good point about 005. It's actually not just extended-isupport that sends it asynchronously, it's Modern:
If the value of a parameter changes, the server SHOULD re-advertise the parameter with the new value in an RPL_ISUPPORT reply. An example of this is a client becoming an IRC operator and their CHANLIMIT changing.
I think I'm fine with consolidating everything into the numerics.
There was a problem hiding this comment.
Actually, thoughts on consolidating everything into METADATA instead? This would avoid having to echo the user's own nick on everything, as is conventional for numerics.
There was a problem hiding this comment.
(Never mind, this has slightly more problematic compatibility implications and doesn't seem worth it)
| unprefixed `metadata-2` capability name. | ||
| Instead, implementations SHOULD use the `draft/metadata-2` capability name to be interoperable with other | ||
| software implementing a compatible work-in-progress version. | ||
| Software implementing this work-in-progress specification MUST NOT use the unprefixed `metadata` capability name. Instead, implementations SHOULD use the `draft/metadata-2` capability name to be interoperable with other software implementing a compatible work-in-progress version. |
There was a problem hiding this comment.
Thoughts on going up to draft/metadata-3?
There was a problem hiding this comment.
I feel this is mostly backwards compatible despite the large number of changes: command syntax is identical and numeric formats are identical. The changes in standard reply codes are not backwards compatible, except for RATE_LIMITED, which is the only standard reply code that required/suggested the client to automatically act on it. Every other code was informative to alert the user to issues, so changing the code doesn't really change how client software can do this (although it may break GUI notifications looking for previous codes that no longer exist).
Something missing from this list that's in my next set of changes to push up after gathering more feedback is that I made empty strings no longer valid for metadata values, which is another backwards incompatible change. This was originally done before I decided to get rid of the METADATA notification in favor of numerics, as a way of specifying that a notification without a value means the key is not set, but I left the language in there after moving to numerics-only because I couldn't think of any good reasons why a piece of metadata MUST have an empty value instead of the client specifying some random byte to just fill something there.
All that being said, if people would rather this be draft/metadata-3 then I can do that. But in existing client implementations things won't break horribly much if we keep this at draft/metadata-2, I think (this needs additional testing but I'm working on fixing some bugs with my server-side implementation of these changes so I won't be able to test clients for a couple weeks).
There was a problem hiding this comment.
What are the current client implementations?
There was a problem hiding this comment.
Halloy, IRCCloud, Obsidian, Goguma
| * The `RATE_LIMITED` standard reply code is now specified for every subcommand. | ||
| * The `INVALID_PARAMS` standard reply code has been added for all other errors with `METADATA` command parameters. | ||
| * Standard reply codes have been adjusted to use `WARN` and `NOTE` when and where appropriate. `FAIL` is reserved for when the entire command cannot proceed due to an error. `WARN` is used when a portion of a command cannot proceed but other parts may still proceed. `NOTE` is used for ancillary data when the command is fully successful. | ||
| * The `*ALL` target has been specified for `METADATA SYNC` to synchronize all visible targets. |
There was a problem hiding this comment.
Someone on #ircv3 brought up that
*ALL seems weird, I'd proably [sic] go for 0 or % or @ (e.g. '%' in IMAP is a wildcard, and '@' in IRC is an invalid nickname)
There wasn't much in-channel discussion although I noted my distaste for 0 since that historically signals "nothing" instead of "everything". I suggested $* as another potential option, borrowing from the $ prefix used in some ircds for "broadcast" messages. Because of this prefix in that context, modern.ircdocs forbids $ at the start of nicknames, and it is highly unlikely that the IRCv3 WG would standardize a $ channel prefix as that would collide with the broadcast message prefix on those ircds.
There was a problem hiding this comment.
I personally like the idea of * as a non-colliding prefix (perhaps namespace) for special entities / targets, but I'm certainly open to $* here if other people have strong feelings.
- Move CAP to draft/metadata-3 - Add missing change entry that empty values are no longer permitted and are treated as unsetting the key in the Errata list (this was the case in the original version of this PR but was omitted from the Errata listing) - Fix incorrect example citing a `SYNC` target of `*` instead of `*ALL` - Further clarify which CAP tokens should be treated as unlimited if missing from CAP LS 302
The errata section at the bottom lists all normative changes. Additionally, the specification was rearranged and copyedited to flow more nicely when read from top-to-bottom. It should (hopefully) be possible to now follow the specification without needing to jump around, and all concepts are (hopefully) introduced before being used in the specification. Some examples were moved inline to help illustrate, although the rather large examples section at the end was maintained in case it was helpful.
Addresses issues and discussion from #588