Bookmark enhancements
Relevant Issues
Closes #918
Description
This simplifies the bookmark implementation, and adds the ability to delete bookmarks in DMs.
To reduce complexity, the delete bookmark functionality actually allows users to delete any sir-lancebot message within their own DMs, and is labeled as such.
Did you:
- [x] Join the Python Discord Community?
- [x] Read all the comments in this template?
- [x] Ensure there is an issue open, or link relevant discord discussions?
- [x] Read and agree to the contributing guidelines?
@ChrisLovering Enhance faster!
Will you be finishing up this PR sometime soon?
Overall it works nicely! Thanks for working on this! I have a few documentation suggestions to improve user usability, and also I'd like to repeat the help command unideal behaviour.
Using
.helpis a bit confusing since it doesn't work in DMs nor any channel except for #bot-commands.
IMO it's even more confusing if we allow it in DMs, since the Help command only shows the command directly, which makes users think they can run &delete directly
Not too sure what to do here.
IMO it's even more confusing if we allow it in DMs, since the Help command only shows the command directly, which makes users think they can run
&deletedirectly
Looked into that bug and it seems to be because specifying root_aliases causes bookmark delete to be registered as a command rather than subcommand, which causes it to be listed even though subcommands usually aren't, and this isn't done correctly (this isn't specific to DMs).
Tentative patch that seems to fix it:
diff --git a/bot/exts/core/help.py b/bot/exts/core/help.py
index db3c2aa6..f7df0760 100644
--- a/bot/exts/core/help.py
+++ b/bot/exts/core/help.py
@@ -277,7 +277,7 @@ class HelpSession:
else:
results.append(f"<{name}>")
- return f"{cmd.name} {' '.join(results)}"
+ return f"{cmd.qualified_name} {' '.join(results)}"
async def build_pages(self) -> None:
"""Builds the list of content pages to be paginated through in the help message, as a list of str."""
@@ -304,8 +304,9 @@ class HelpSession:
prefix = constants.Client.prefix
signature = self._get_command_params(self.query)
+ paginator.add_line(f"**```\n{prefix}{signature}\n```**")
+
parent = self.query.full_parent_name + " " if self.query.parent else ""
- paginator.add_line(f"**```\n{prefix}{parent}{signature}\n```**")
aliases = [f"`{alias}`" if not parent else f"`{parent} {alias}`" for alias in self.query.aliases]
aliases += [f"`{alias}`" for alias in getattr(self.query, "root_aliases", ())]
aliases = ", ".join(sorted(aliases))
If this seems right I don't mind if you want to kaizen this in here or want me to open a separate PR.
Alternative solutions would be:
- Remove the root alias here
- Add a root alias called
delete - Make the help command not display subcommands with root aliases
I've gone ahead and applied @wookie184's suggested fix and rebased onto main.
Force push is to rebase onto main
