Skip to content

Add extended-isupport #543

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

Merged
merged 4 commits into from
Apr 29, 2025
Merged

Add extended-isupport #543

merged 4 commits into from
Apr 29, 2025

Conversation

Image for: Conversation
Copy link
Contributor

Proposal for ircv3/ircv3-ideas#124

Copy link
Contributor

Rather than adding more numerics can we just require that it gets batched?

Copy link
Contributor Author

Yeah, that would be a cleaner solution. I wonder if this would be hard for older servers to implement?

emersion force-pushed the extended-isupport branch from b098736 to 5b75839 Compare July 12, 2024 17:01
Copy link
Contributor Author

Switched to a batch. Does this look good?

emersion force-pushed the extended-isupport branch 2 times, most recently from 57e7f21 to a8eb127 Compare July 12, 2024 17:05
emersion force-pushed the extended-isupport branch from a8eb127 to 0375eea Compare July 12, 2024 17:50
Copy link
Contributor

progval commented Jul 12, 2024

Could you mention that:

  1. as before, ISUPPORT messages are additive (ie. if a new ISUPPORT overwrites or adds values, but the absence of a value does not mean it's not present anymore)
  2. specify the - prefix to remove a key? I don't think this existed prior to this spec.
Copy link
Contributor Author

as before, ISUPPORT messages are additive (ie. if a new ISUPPORT overwrites or adds values, but the absence of a value does not mean it's not present anymore)

Not sure how to phrase this. Maybe:

As usual, servers can update or delete existing values by sending additional RPL_ISUPPORT messages in a draft/isupport batch after the initial batch.

But maybe you want something more precise?

specify the - prefix to remove a key? I don't think this existed prior to this spec.

It's in modern: https://modern.ircdocs.horse/#rplisupport-005

Copy link
Contributor

SadieCat commented Jul 13, 2024

specify the - prefix to remove a key? I don't think this existed prior to this spec.

It's in modern: https://modern.ircdocs.horse/#rplisupport-005

Also worth noting that Modern gets it from from the original IETF draft isupport specs.

Sending a change:

S: :irc.example.org BATCH +asdf draft/isupport
S: @batch=asdf :irc.example.org 005 * CHANNELLEN=64 -NETWORK
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-NETWORK isn't a very realistic example. Also, maybe show a key being changed via removal and readdition. Also these examples could do with the full CAP negotiation being shown, including CAP END and maybe the 001 being sent.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intended meaning here is that users will get updates before connection registration?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the last part refers to the previous example sorry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I wasn't sure how far the examples should go, I wouldn't want to list the full registration burst. Will add.

-NETWORK could happen if the network configuration file is reloaded with the network name removed. Do you have another example in mind which might be more realistic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe show a key being changed via removal and readdition.

The ISUPPORT spec says that changing a key is done by advertising it again, rather than removing it and then re-adding it.

emersion force-pushed the extended-isupport branch from 0375eea to e1aea37 Compare August 1, 2024 20:43
Copy link
Contributor Author

emersion commented Aug 1, 2024

I've tried to address the comments. Let me know what you think!

Copy link
Member

jwheare commented Aug 2, 2024

Changes look good. No need to force push updates, makes it a bit harder to review. I squash commits on merge anyway.

Copy link
Contributor Author

emersion commented Aug 4, 2024

Alternatively, instead of spec'ing that servers must send the ISUPPORT list when the cap is enabled, we could guarantee that VERSION works pre-reg and recommend that clients send it after enabling the cap.

Copy link
Contributor

slingamn commented Aug 4, 2024

I would be slightly more comfortable with allowing VERSION pre-registration --- the "edge-triggered" API has always bothered me a little (on the other hand, I haven't been able to identify any concrete problems with it).

VERSION has a wrinkle where you also get RPL_VERSION, so presumably you would get RPL_VERSION outside the batch and then a batch of RPL_ISUPPORT? We could add a command like ISUPPORT that just sends the RPL_ISUPPORT.

Copy link
Member

jwheare commented Aug 4, 2024

Just specifying an ISUPPORT command that can work pre-reg works for me. Regarding batch, I feel like this was already discussed, but what use case does batch enable? My implementation just incrementally updates the ISUPPORT list whenever an 005 is received, I don't have a need for an end of isupport marker.

Copy link
Contributor

slingamn commented Aug 4, 2024

@jwheare there is a note on this here: ircv3/ircv3-ideas#124 (comment)

tl;dr it allows the UI to be updated atomically, reducing "flicker".

Copy link
Member

jwheare commented Aug 4, 2024

Ok so maybe define the ISUPPORT command to require use of a batch if the cap is enabled? But leave the existing automatic isupport sent post reg unbatched. Would that work?

Copy link
Contributor Author

That wouldn't work if the server wants to send an atomic update of more than 13 tokens.

Copy link
Contributor Author

A new ISUPPORT command has been introduced, and the RPL_ISUPPORT list is no longer sent when the capability is enabled.

Copy link
Contributor

Looks good, I merged support for this in ergo and deployed it on my staging server ircs://files.stronghold.network

Copy link
Contributor Author

I've finally got some time to implement this in Goguma! Here's the patch: https://codeberg.org/emersion/goguma/pulls/2

Ergo's implementation seems to have a small bug: there is an extraneous chathistory param in the batch parameters.

flutter: [3] <- :files.stronghold.network BATCH +1 chathistory draft/extended-isupport
flutter: [3] <- @batch=1 :files.stronghold.network 005 * AWAYLEN=500 BOT=B CASEMAPPING=ascii CHANLIMIT=#:100 CHANMODES=Ibe,k,fl,CEMRUimnstu CHANNELLEN=64 CHANTYPES=# CHATHISTORY=100 ELIST=U EXCEPTS EXTBAN=,m FORWARD=f INVEX :are supported by this server
flutter: [3] <- @batch=1 :files.stronghold.network 005 * KICKLEN=1000 MAXLIST=beI:60 MAXTARGETS=4 MODES MONITOR=100 MSGREFTYPES=msgid,timestamp NETWORK=OraStage NICKLEN=32 PREFIX=(qaohv)~&@%+ STATUSMSG=~&@%+ TARGMAX=NAMES:1,LIST:1,KICK:,WHOIS:1,USERHOST:10,PRIVMSG:4,TAGMSG:4,NOTICE:4,MONITOR:100 TOPICLEN=1000 UTF8MAPPING=rfc8265 :are supported by this server
flutter: [3] <- @batch=1 :files.stronghold.network 005 * UTF8ONLY WHOX draft/CHATHISTORY=100 :are supported by this server
flutter: [3] <- :files.stronghold.network BATCH -1

With a small workaround for the broken batch type, it seems to work fine:

Copy link
Contributor

Sorry about that. I fixed it in ergochat/ergo#2197 and redeployed.

Copy link
Contributor Author

emersion commented Nov 3, 2024

The batch is named "draft/isupport", not "draft/extended-isupport".

Copy link
Contributor Author

Gentle ping :)

Copy link
Member

jwheare commented Apr 29, 2025

Fine to merge this as is, but would the non draft cap name be more self explanatory as prereg-isupport? The format is not being extended by this afaict.

Copy link
Contributor Author

The name "extended-isupport" was picked because the cap also introduces a new batch and a new command. I'm fine using any other name if you prefer.

Copy link
Member

jwheare commented Apr 29, 2025

early-isupport was also suggested by @SadieCat to avoid the potentially confusing "registration" term being used. Anyway, no need to change the draft cap, but merged version should be. Hopefully someone remembers when the time comes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants