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

added /requests command #3746

Merged
merged 4 commits into from
May 22, 2022
Merged

added /requests command #3746

merged 4 commits into from
May 22, 2022

Conversation

badoge
Copy link
Contributor

@badoge badoge commented May 17, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Opens the current channel's reward queue, can also take an input to open some other channel's queue.

zneix
zneix previously requested changes May 17, 2022
Copy link
Collaborator

@zneix zneix left a comment

Choose a reason for hiding this comment

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

I'm sorry but I'm going to deny this one. With /openurl and the extended placeholders that were added in #3155 you can create many creative commands like this one on your own, e.g.:
c2 commands 101

I just don't want extra code for something that doesn't need it at all.

P.S. I see that the thing this PR has and current implementation of placeholders don't is different behaviour when no arguments are provided.
Sure, at this point there's no default/fallback values but I believe it could be implemented in the near future, so that you could use /requests in #forsen to open forsen's requests or use /requests pajlada also in #forsen to open pajlada's requests.

@zneix
Copy link
Collaborator

zneix commented May 17, 2022

On the other note, perhaps a little section with "examples" of how command placeholders can be used might be a cool addition - this PR alone being created in the first place shows that it is not as commonly known that these super useful placeholders exist. Just to show users that they can have something without much effort and show them how to do that.

@zneix zneix closed this May 17, 2022
@badoge
Copy link
Contributor Author

badoge commented May 17, 2022

I added it because it's a command in webchat https://help.twitch.tv/s/article/chat-commands?language=en_US#AllMods

i thought chatterino was meant to match as many default twitch commands as possible FeelsDankMan

@zneix
Copy link
Collaborator

zneix commented May 17, 2022

I added it because it's a command in webchat help.twitch.tv/s/article/chat-commands?language=en_US#AllMods

Oh, this is a completely different reasoning then. I had 0 Clue that such command exists in webchat. If it does then sure, we can add this. (Although it doesn't show up in .help's output at all, weird.

Sorry for the unnecessary ruckus.

@zneix zneix reopened this May 17, 2022
@Mm2PL
Copy link
Collaborator

Mm2PL commented May 17, 2022 via email

Mm2PL
Mm2PL previously requested changes May 17, 2022
Copy link
Collaborator

@Mm2PL Mm2PL left a comment

Choose a reason for hiding this comment

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

Change the brackets. Might be good to add this into the commands list on the wiki: https://github.com/Chatterino/wiki/blob/master/docs/Commands.md

src/controllers/commands/CommandController.cpp Outdated Show resolved Hide resolved
Co-authored-by: Mm2PL <mm2pl+gh@kotmisia.pl>
@Felanbird
Copy link
Collaborator

Change the brackets. Might be good to add this into the commands list on the wiki: https://github.com/Chatterino/wiki/blob/master/docs/Commands.md

I was thinking of this, but since it's a webchat command I didn't mention it, since we mainly went with commands that have some sort of separate functionality for that wiki page.

@pajlada pajlada dismissed stale reviews from zneix and Mm2PL May 22, 2022 11:31

Fixed

@pajlada pajlada enabled auto-merge (squash) May 22, 2022 11:32
@pajlada pajlada merged commit bd3d2ed into Chatterino:master May 22, 2022
@badoge badoge deleted the requests branch July 4, 2022 03:21
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.

None yet

5 participants