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 -remove command #71

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add -remove command #71

wants to merge 2 commits into from

Conversation

michael-je
Copy link
Owner

@michael-je michael-je commented Jan 6, 2022

Adds the -remove command, which allows for removing specific songs from the queue.

Also:

  • prepends www. to the docs url
  • changes the -help command slightly by adding an extra whitespace between the command names and their descriptions. I Did this because one of the command names was touching its description, which was quite ugly.

Closes #68

@michael-je
Copy link
Owner Author

michael-je commented Jan 6, 2022

@michael-je michael-je added the enhancement New feature or request label Jan 7, 2022
Copy link
Collaborator

@morgaesis morgaesis left a comment

Choose a reason for hiding this comment

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

Nice PR with nice UI :)

Some comments, which are mostly suggestions/rfc.

However, there are no tests. I won't 👍 until tests are in place.

@@ -91,7 +91,7 @@ class MusicBot:

COMMAND_PREFIX = "-"
REACTION_EMOJI = "👍"
DOCS_URL = "github.com/michael-je/the-lone-dancer"
DOCS_URL = "www.github.com/michael-je/the-lone-dancer"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this necessary? In order to get a clickable link this needs to start with https:// and can't be in a formatted code block. Making the doc url a hyperlink also causes url preview below the message, which I'm not a fan of.

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's not necessary, I just don't think that the original properly signals that it's a link. But github actually doesn't use the www. prefix, so https:// is probably more appropriate.

Links inside code blocks don't become hyperlinked.

bot.py Show resolved Hide resolved
bot.py Show resolved Hide resolved
bot.py Show resolved Hide resolved
bot.py Show resolved Hide resolved
bot.py Show resolved Hide resolved
@michael-je
Copy link
Owner Author

I'll also add some tests! It's about time I get my hands dirty with mocking 😨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add sub-commands to -clear
2 participants