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

[WIP] Channel renaming extension. #308

Closed
wants to merge 3 commits into from
Closed

[WIP] Channel renaming extension. #308

wants to merge 3 commits into from

Conversation

SadieCat
Copy link
Contributor

@SadieCat SadieCat commented Jun 3, 2017

Copy link
Member

@jwheare jwheare left a comment

Choose a reason for hiding this comment

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

👍 looks good. Maybe a non normative suggestion for servers to optionally apply temp forwarding from old chan to new chan to aid migration for users offline when the rename happened?

Is there a reference implementation for inspircd?

@SadieCat
Copy link
Contributor Author

SadieCat commented Jun 4, 2017

Is there a reference implementation for inspircd?

There will be soon™. I just need to finish up a few things.

@Zarthus
Copy link

Zarthus commented Jun 4, 2017

I don't see anything wrong, but I feel like rename is kind of generic of a keyword and won't really scale if you ever expand this idea. Let's say in the future you want to tack on the ability to archive (read-only) channels or close them down entirely. To be consistent with this format you'd need to reserve ARCHIVE and CLOSE, and /close already is an alias in many IRC clients.

I would probably do something like CHGCHAN (consistent with CHGHOST) or CHANRENAME, and I agree it is bikeshedding to some degree, but it's something that should at least be mentioned if you want to expand this idea to the degree Slack already has.

extensions/rename.md Outdated Show resolved Hide resolved
@Zarthus
Copy link

Zarthus commented Jun 4, 2017

Would it be possible to rename a channel from #chan to !chan? On some IRCds this means the channel serves a different function and clients may not wish to adhere to channel redirection (let's say #chan is a normal channel and !chan is opless mode which does not support founders).

Awfully ircd specific, but perhaps worth mentioning in a security or nonnormative section.

@RyanSquared
Copy link
Contributor

Would it be possible to rename a channel from #chan to !chan?

To quote @SaberUK from IRC:

2017-06-04 09:23:12 SaberUK Zarthus: i'm thinking it should be a SHOULD NOT for servers

@DanielOaks
Copy link
Member

If a more specific error like ERR_CHANOPRIVSNEEDED can be sent in response to a rename request, what should servers do?

Should they send both ERR_CHANOPRIVSNEEDED followed by ERR_CANNOTRENAME, and similar for ERR_CHANNAMEINUSE. It seems appealing for clients/bots to be able to expect either a RENAME command or a ERR_CANNOTRENAME numeric in response, to know for sure whether it failed or succeeded.

Plays into the questions brought up in IRC around how to alert for errors with specific commands in general going forward. I'm just using the existing ERR_UNKNOWNERROR in my hacked-up impl, which includes the original sent command in it.

Server implementations MUST implement a fallback method for legacy clients.
This method SHOULD involve sending a fake `PART` message for the old channel
name and a `JOIN` message for the new channel name as well as a `MODE` message
to restore any prefix modes that the legacy client has applied to it.
Copy link
Member

@DanielOaks DanielOaks Jun 5, 2017

Choose a reason for hiding this comment

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

It seems like this should be specified in greater detail as a normative thing. Should the PART message be the reason specified by the sending user, and include the name of the user who sent the rename request? It seems like they should also send the NAMES message and TOPIC ones if they normally send those, or it could even say something like 'just send everything as though it were a regular channel join' perhaps?

Specifying this in more detail might help reduce any possible confusion around here, and make sure servers stay consistent with how they implement it.

Copy link
Contributor

Choose a reason for hiding this comment

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

As with the MODE for sending prefix modes, what about modes that exist otherwise, such as prefix ones such as +b and +f (ban and forward) and all other possible channel modes?

Copy link
Member

Choose a reason for hiding this comment

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

I'd probably just send everything you'd normally send on a client joining the channel. Name lists, channel modes and all. If you'd normally send it, cool. If not, I'd leave it out.

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 standard channel join burst is definitely required here, NAMES, TOPIC, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, will push shortly.

@SadieCat
Copy link
Contributor Author

SadieCat commented Jun 6, 2017

If a more specific error like ERR_CHANOPRIVSNEEDED can be sent in response to a rename request, what should servers do?

Yeah, thats what my implementation sends. I'll amend the spec in a bit.

@SadieCat
Copy link
Contributor Author

I think that should be everything other than deciding what numerics are to be used.

@jwheare jwheare added this to the Roadmap milestone Aug 9, 2017
extensions/rename.md Outdated Show resolved Hide resolved
## Implementation Considerations

Server implementations MUST implement a fallback method for legacy clients.
This method SHOULD involve sending the legacy client a `PART` message for the
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the PART reason should contain the RENAME reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, i'll add that.

Copy link

Choose a reason for hiding this comment

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

shouldn't it also contain the new channel name?

Copy link
Member

@jwheare jwheare left a comment

Choose a reason for hiding this comment

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

Cool, I think anything in "considerations" sections that involves normative MUST, SHOULD or MAY directives would be better further up in an "Details" section, above examples.

I feel like considerations are usually for non-normative stuff, and implementation details, where I would use lowercase "might" instead of MAY.

Even the MAY stuff in here is probably normative rather than a consideration. E.g. specific error numerics reserved for moderation/permission stuff MUST be used but only if the server is actually performing the validation in question (it MAY choose not to).

extensions/rename.md Outdated Show resolved Hide resolved
extensions/rename.md Outdated Show resolved Hide resolved
extensions/rename.md Outdated Show resolved Hide resolved
extensions/rename.md Outdated Show resolved Hide resolved
@jesopo
Copy link

jesopo commented Nov 5, 2018

Implemented in BitBot: bitbot-irc/bitbot@6d742f6a

@RyanSquared
Copy link
Contributor

Implemented (minimally) in Moon Moon: wiseguiz/Moon-Moon@504e354

@DanielOaks
Copy link
Member

This is implemented in Oragono (has been since 0.8.1 - 2017-06), but there's been a faaair few spec updates since then, will update to match the latest state of the PR.

@jwheare
Copy link
Member

jwheare commented Mar 6, 2019

Implemented in IRCCloud too. Spec still needs some adjustment, my prior review requesting changes still stands fwict.


## Implementation Considerations

Server implementations MUST implement a fallback method for legacy clients. This method MUST involve sending the legacy client a `PART` message for the old channel name, with the rename reason as the part message if given, followed by the usual messages that would be sent if the legacy client had joined the new channel normally (`JOIN`, `RPL_TOPIC`, `RPL_NAMREPLY`, etc) and finally a `MODE` message to restore any prefix modes that the legacy client has applied to it.
Copy link
Member

@jwheare jwheare Mar 11, 2019

Choose a reason for hiding this comment

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

Prompted by ergochat/ergo#300 (comment)

MODE isn't needed here. Prefix modes come from NAMES, and the channel mode is not normally included in a join burst, clients will request it on join.

(Edit to add a one-click suggestion)

Suggested change
Server implementations MUST implement a fallback method for legacy clients. This method MUST involve sending the legacy client a `PART` message for the old channel name, with the rename reason as the part message if given, followed by the usual messages that would be sent if the legacy client had joined the new channel normally (`JOIN`, `RPL_TOPIC`, `RPL_NAMREPLY`, etc) and finally a `MODE` message to restore any prefix modes that the legacy client has applied to it.
Server implementations MUST implement a fallback method for legacy clients. This method MUST involve sending the legacy client a `PART` message for the old channel name, with the rename reason as the part message if given, followed by the usual messages that would be sent if the legacy client had joined the new channel normally (`JOIN`, `RPL_TOPIC`, `RPL_NAMREPLY`, etc).

@ilbelkyr
Copy link

ilbelkyr commented Apr 3, 2019

As mentioned on IRC, it's probably a good idea to explicitly mention the possibility of a RENAME message that only changes casing to ensure this is properly handled by clients.

I feel like the requirement to provide a PART/JOIN fallback might be relaxed in this situation since it may be more disruptive than silently starting to use a different casing in messages from the server, although that's certainly debatable.

@RyanSquared
Copy link
Contributor

I feel like the requirement to provide a PART/JOIN fallback might be relaxed in this situation since it may be more disruptive than silently starting to use a different casing in messages from the server, although that's certainly debatable.

I think this would break clients that don't care about casing (such as my bot), whereas the PART/JOIN solution would most likely work on all clients.

@SadieCat
Copy link
Contributor Author

I think this would break clients that don't care about casing (such as my bot)

Such software is already broken as there is no guarantee the server will not change the case of a channel name (e.g. InspIRCd has a changecap module in extras).

@jwheare
Copy link
Member

jwheare commented Apr 27, 2020

@SadieCat let me know if you want me to take this over to tidy it up? Not sure if it already allows edits from maintainers.

@dequis
Copy link
Contributor

dequis commented Apr 27, 2020

It says "from unknown repository" so I think you gotta close this PR and open a new one with a new source repo.

@jwheare
Copy link
Member

jwheare commented Jun 8, 2020

Replaced with #420 (blaze it)

@jwheare jwheare closed this Jun 8, 2020
hhirtz added a commit to hhirtz/oragono that referenced this pull request Aug 4, 2020
See
<https://github.com/ircv3/ircv3-specifications/pull/420/files#diff-70e90beef48dc9cf5d784d1e179ea822R42>

Moreover, make it so that clients that haven't negotiated `draft/rename`
don't receive JOIN and PART messages. Rationale:
<ircv3/ircv3-specifications#308 (comment)>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants