-
Notifications
You must be signed in to change notification settings - Fork 78
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
Add rewritten metadata specification #250
Conversation
subscribe to. | ||
|
||
Servers and clients MAY support both of the mentioned extensions, but MUST NOT | ||
negotiate both of them at the same time in the same connection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on the order, I'd allow the client to either request both (metadata-notify
and then ...-2
) or allow ...-2
and reject the other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any good reason to do it that way? I've deliberately tried to avoid adding special cases like "if both are present, accept only one" or "if the order is X Y accept if Y X then reject". Servers must accept all caps in a CAP REQ or reject them all per the cap spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By 'order' here I mean specifically each capability is in a REQ
of its own; it doesn't make sense to reject ...-2
if metadata-notify
was negotiated previously.
Clients MAY request
metadata-notify-2
after a successfulmetadata-notify
.
...may be the best way to phrase it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any good reason to deliberately allow that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...Well, if a client author's developing support for IRCv3.3 metadata, they might have already negotiated metadata-notify
by default. It's not a particularly good reason, though, so I won't insist on it.
The key subscription needs to be disclosed to the client somewhere, e.g. via a |
I don't exactly understand what you mean. Can you elaborate? If this is about advertising the availability of the new subcommands, the presence of |
Sorry, I missed a word there; the key subscription limit needs to be disclosed. |
I agree, that should be advertised. I think it should be in the cap's value because we can't change the format of the value of the METADATA ISUPPORT token after release and adding a new ISUPPORT token is not necessary if there is a cap already. |
I disagree; the |
Changing the ISUPPORT format only if the CAP is negotiated seems reasonable. |
Clients released before this specification can't figure out "which version is which" because they expect a single integer as the value of the ISUPPORT token.
Clients can request metadata-notify-2 after ISUPPORT (say after they receive a CAP NEW) and they won't get the value that way. Also existing servers have a cached ISUPPORT and send that to all clients. Having per client ISUPPORT is something they'd have to add support for separately for this case and it has no benefit over advertising as a cap value. (In fact, the cap value has benefits as it allows changing the limit on the fly for example). |
This is a valid criticism.
This is a terrible idea. |
97578e3
to
5de8f8f
Compare
What's the status of this? Do we need implementations for this to move forward? |
Yes, and I think it should be merged with #245 so we can point to a single PR from the tracker issue. |
Somewhat experimental implementation here: https://github.com/attilamolnar/inspircd/tree/master%2Bmetadata |
4e13133
to
4ac53c7
Compare
4ac53c7
to
d8dda9a
Compare
More ideas: Remove the |
Closing in favour of #339 |
TODO:
Discuss:
metadata
(no need for version number, values are about more than notifications)