feat: context menu for !raw #2773
Conversation
VirdanTheBurden
added
a: utility
bot/exts/info/information.py
Outdated
| @@ -480,19 +485,29 @@ def format_fields(self, mapping: Mapping[str, Any], field_width: int | None = No | |||
| # remove trailing whitespace | |||
| return out.rstrip() | |||
|
|
|||
| async def send_raw_content(self, ctx: Context, message: Message, json: bool = False) -> None: | |||
| async def send_raw_content(self, ctx: Context | Interaction, message: Message, json: bool = False) -> None: | |||
There was a problem hiding this comment.
I think this function should be simplified into just a function that accepts a message, and returns a str or list[str] (to accommodate for pagination). Accepting both a Context and Interaction feels a bit janky in my opinion.
There was a problem hiding this comment.
I didn't feel quite the same way at first, but due to other reasons, this route of implementation is frankly garbage. I'll be moving the message sending responsibilities to the... overlying(?) methods.
There was a problem hiding this comment.
I'll be moving the message sending responsibilities to the... overlying(?) methods.
That sounds like a good idea, now that we have two different methods that send the message in two different ways (normal !raw command and context-menu command. Former needs to use ctx.send while the latter needs to use interaction.response.send_message). It would make sense to practice principle of least responsibility here and have one function that's just responsible for fetching the raw content.
There was a problem hiding this comment.
I've pushed some new changes that made send_raw_content only fetch and return a Paginator object instead. Feel free to take a look at the new commits while I go fix the tests. I think they're borked with the ContextMenu being there in the __init__.
There was a problem hiding this comment.
Er, take that back. Tests passed with flying colors, haha!
After Robin's suggestion, I've realized making send_raw_content manage building the paginator and message sending responsibilties is silly. Thus, send_raw_content is now only responsible for constructing the response message. To reflect this change, the signature for send_raw_content is now get_raw_content, and no longer requires a discord.ext.commands.Context object to work. The only other change needed was moving a permission check to the !raw command coroutine. It cannot exist inside get_raw_content without the context of the author who sent the !raw command.
VirdanTheBurden
force-pushed
the
context-menu
branch
from
c0aa7d1
to
cf8ae34
Compare
Added the context menu object to the command tree and finished the callback. The only thing iffy about it is this: due to the nature of interactions, only one message can be sent as a response to an interaction, with followup webhooks needed for further response. `paginator.pages` is at LEAST 2 elements long, meaning I can only send either element 0 (the "header") or element 1, which is the ACTUAL raw content. I opted to only send element 1. I did try followups in order to send both, but I felt the result was janky, especially since I implemented the response to be ephemeral. Also imported Interaction object in the `from discord import ...` for easier typehinting.
VirdanTheBurden
added
s: needs review
| async def _raw_context_menu_callback(self, interaction: Interaction, message: Message) -> None: | ||
| """The callback to be ran when the Raw Text context menu button is used.""" | ||
| paginator = await self.get_raw_content(message) | ||
| await interaction.response.send_message(paginator.pages[1], ephemeral=True) |
There was a problem hiding this comment.
Perhaps we could trial out some button-based pagination here? It's a shame that users can only see one page with the context menu version. I think one of the largest (pun intended) issues people had with buttons for pagination is their large vertical size, though it may be less of an issue in ephemeral messages.
There was a problem hiding this comment.
Yeah, okay. I'll try adding a view with some buttons to see if we can scroll through.
There was a problem hiding this comment.
An alternative would be to do what was suggested in #1037 and just send a pastebin link if the message is too long, maybe in a similar way to the eval command where it displays as much as it can first.
wookie184
added
s: waiting for author
Closes Issue #2760.
The
!rawcommand has been restricted to only prefix command usage since inception, and has not been touched since. With the introduction of new interfaces for executing commands (i.e. application context menus) and the inconvenience of copying message links to check their raw content, this pull request hopes to simplify the process by adding a context menu button for users to click.