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

Add support for opening usercards by ID #4934

Merged
merged 6 commits into from
Nov 6, 2023
Merged

Conversation

Mm2PL
Copy link
Collaborator

@Mm2PL Mm2PL commented Nov 2, 2023

Description

Syntax is: /usercard #117691339.

Technically maybe enables @#117691339 to be a link to my user if it ever matches.

Copy link
Contributor

@kornes kornes left a comment

Choose a reason for hiding this comment

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

one issue, but works good 👍

Copy link
Contributor

@Nerixyz Nerixyz left a comment

Choose a reason for hiding this comment

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

When entering a nonexistent ID, the ID will display in the name with the hash. But this isn't any different from before, so not an issue. 👍

src/widgets/dialogs/UserInfoPopup.cpp Outdated Show resolved Hide resolved
Co-authored-by: nerix <nerixdev@outlook.de>
@pajlada
Copy link
Member

pajlada commented Nov 2, 2023

I would rather see this be a separate command, similar to /banid

@Felanbird
Copy link
Collaborator

I disagree realistically banid should have never been a separate command anyway

@pajlada
Copy link
Member

pajlada commented Nov 2, 2023

Second attempt: I dislike the # prefix - it's already used for channels for IRC. Can we be explicit and use id:? (e.g. /usercard id:11148817)

@Felanbird
Copy link
Collaborator

Yeaaaaaaaaaah, it's unfortunate that IRC uses # since it's seems obviously correct for #USERID

@Mm2PL
Copy link
Collaborator Author

Mm2PL commented Nov 2, 2023

I could actually allow both prefixes and not complicate the code that much. As for it being a separate command, that could work although it's not something I'd like. As felanbird said, it shouldn't need to be a separate command.

@pajlada
Copy link
Member

pajlada commented Nov 5, 2023

#4945 implements this for /ban and /timeout using id: prefix, with unit tests so it can be re-used for further commands (like this one) in the future.
I don't think allowing both id: and # is a good idea, so just id: as done in my PR above is my preferred way forward

@pajlada pajlada enabled auto-merge (squash) November 6, 2023 17:54
@pajlada pajlada disabled auto-merge November 6, 2023 19:42
@pajlada pajlada merged commit f943f70 into master Nov 6, 2023
14 of 15 checks passed
@pajlada pajlada deleted the feature/usercard_by_id branch November 6, 2023 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants