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

SETNAME Spec #361

Merged
merged 34 commits into from
Apr 2, 2019
Merged

SETNAME Spec #361

merged 34 commits into from
Apr 2, 2019

Conversation

justjanne
Copy link
Contributor

In Short

  • Introduces a new setname capability
  • Introduces a new bidirectional SETNAME command

Rationale

Realnames have historically been immutable for a connection, which has
historically been accepted, as they were rarely used and usually not as present
in client UIs. Still, multiple IRCds have provided independent, non-standard
commands to update realnames.

Nowadays, multiple clients show realnames directly in chat, next to the nick
name, or use information from the realname to provide enhancements, such as
using emails in the realname to provide gravatar based avatars, leading to
higher demand for this functionality than ever.

extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
@DanielOaks
Copy link
Member

Nice, I like it.

The draft/setname capability. I presume that the s2c SETNAME message will only be sent to clients that have negotiated that capability? (and clients that have not negotiated it will not receive the SETNAME message).

With servers validating the new realname, if a given realname is too long it feels like the server should probably just silently truncate it, matching what servers already do when getting it initially via USER.

If the server would otherwise reject the change (maybe because the client's been changing it too fast, it's being used to bypass a channel ban like how nick changes are restricted, or the value's just bad), it might make sense to explicitly send a response. Maybe a good chance to use the proposed FAIL message, something like this:

FAIL SETNAME CANNOT_CHANGE_REALNAME :Slow down your realname changes
FAIL SETNAME CANNOT_CHANGE_REALNAME :Cannot change realname while banned from a channel
FAIL SETNAME INVALID_REALNAME :Realname is not valid

@jesopo
Copy link

jesopo commented Feb 13, 2019

Very simple implementation in BitBot as of bitbot-irc/bitbot@acbe02ba - let me know when there's a test server to try it on

extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
@DanielOaks
Copy link
Member

DanielOaks commented Feb 13, 2019

PR written up for Oragono here, pending the review q above right now it fails if the sending user hasn't negotiated the cap Updated the implementation to match intended behaviour, the spec language should explicitly state the intended behaviour.

@jwheare
Copy link
Member

jwheare commented Feb 13, 2019

Should there be a NAMELEN isupport key?

@DanielOaks
Copy link
Member

DanielOaks commented Feb 13, 2019

On how existing impls handle the realname being too long in their current SETNAME commands:

Insp:

user->WriteNotice("*** SETNAME: Real name is too long");
maxreal is 128 by default

Unreal:

sendnotice(sptr, "*** /SetName Error: \"Real names\" may maximum be %i characters of length", REALLEN);
REALLEN is 50

So they both explicitly send a notice to the user and fail out, not changing the realname. Cool.

@Zarthus
Copy link

Zarthus commented Feb 13, 2019

Does it make sense to call this SETNAME over SETREALNAME or SETGECKOS?

When I think of setname I think of /nick, personally. Alternatively to be consistent with /nick you could just call it /realname too

To add on Dan's point: charybdis silently truncates realnames at 32/64 characters I think.

@DanielOaks
Copy link
Member

DanielOaks commented Feb 13, 2019

/SETNAME is already in use by Insp and Unreal, if this could be compatible and use the same command name that'd be cool. If we would invent our own from scratch though I'd def go for /REALNAME to be consistent with /NICK ;)

@jesopo
Copy link

jesopo commented Feb 13, 2019

+1 for the command to be REALNAME not SETNAME

@jwheare
Copy link
Member

jwheare commented Feb 13, 2019

I'd favour not changing a command that users might be accustomed to just for the sake of it.

@jwheare
Copy link
Member

jwheare commented Feb 13, 2019

This is now deployed on the IRCCloud client, and our teams/slack/bnc servers. @jesopo you should already have access to the testing team server.

extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
@DanielOaks
Copy link
Member

Looks good to me, nice work.

Co-Authored-By: justjanne <janne@kuschku.de>
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.

Some change suggestions.

extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
extensions/setname.md Outdated Show resolved Hide resolved
jwheare and others added 14 commits March 29, 2019 13:26
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
Co-Authored-By: justjanne <janne@kuschku.de>
@jwheare
Copy link
Member

jwheare commented Mar 29, 2019

Thanks, this is mergeable as draft now imo.

@jwheare jwheare merged commit 09b1ad2 into ircv3:master Apr 2, 2019
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.

None yet

7 participants