Skip to content

Help command fix, normalize suggestions for unknown commands#1064

Merged
Xithrius merged 15 commits into
mainfrom
help_command_fix_863
Aug 18, 2022
Merged

Help command fix, normalize suggestions for unknown commands#1064
Xithrius merged 15 commits into
mainfrom
help_command_fix_863

Conversation

@RohanJnr
Copy link
Copy Markdown
Contributor

Relevant Issues

issue: #863

Refer to #884

Fixes

  • The help command used to raise an error when provided with an invalid command name (fixed)
  • There used to be different command suggestions from the help command and the error handler when an unknown command was provided.
    • This has been fixed by using the same method for finding similar commands.

Changes

  • Providing an invalid subcommand but valid parent command (for example: !ext listt, ext is valid but subcommand listt is invalid) now sends the help command output for the parent command.

  • Using bot.all_commands attribute instead of bot.walk_commands() generator.

    • walk_commands() is not useful in the error_handler because (as it provides us with all subcommands also), when there is an invalid subcommand, the parent command (commands.Group) handles this case and sends a help message, hence, never reaching the error_handler.
    • all_commands does not give us subcommands (it gives aliases which is pretty useful).
  • Using rapidfuzz for finding similar commands instead of difflib (reasons being speed).

  • Error handler now sends multiple command suggestions instead of 1 when user is trying to run an unknown command.

Yet to decide/to discuss

The help command lists/suggests commands that may not in the user's power to invoke them. Should we filter commands when listing them such that to only show them the commands which they can invoke ?

This is the current output when user invokes help command on a restricted command.
Uploading image.png…

Did you:

@RohanJnr RohanJnr requested a review from Bluenix2 June 22, 2022 03:16
Comment thread bot/exts/core/error_handler.py Outdated
Comment thread bot/exts/core/error_handler.py Outdated
Comment thread bot/exts/core/help.py Outdated
Comment thread bot/exts/core/help.py Outdated
@Xithrius Xithrius added area: backend Related to internal functionality and utilities status: needs review Author is waiting for someone to review and approve type: enhancement Changes or improvements to existing features category: utilities Related to utilities labels Jun 23, 2022
@RohanJnr RohanJnr requested a review from Bluenix2 June 23, 2022 14:00
@Xithrius Xithrius requested review from brad90four and ichard26 June 25, 2022 22:23
Copy link
Copy Markdown
Contributor

@brad90four brad90four left a comment

Choose a reason for hiding this comment

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

Content wise this looks good to me, have not verified with independent testing yet.

@Xithrius Xithrius requested review from ChrisLovering and GDWR and removed request for ichard26 July 9, 2022 19:31
@Xithrius Xithrius enabled auto-merge (squash) July 9, 2022 19:32
GDWR
GDWR previously requested changes Jul 9, 2022
Copy link
Copy Markdown
Contributor

@GDWR GDWR left a comment

Choose a reason for hiding this comment

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

Looks good, just a slight type hint that could catch somebody out.

Comment thread bot/exts/core/help.py Outdated
@ichard26
Copy link
Copy Markdown
Contributor

I'm in the middle of a review (sorry for taking so long!) but I ran out of time to finish it today :/ I'll submit my review tomorrow.

@ichard26 ichard26 self-requested a review July 10, 2022 02:57
@RohanJnr RohanJnr requested a review from GDWR July 11, 2022 04:08
Copy link
Copy Markdown
Contributor

@GDWR GDWR left a comment

Choose a reason for hiding this comment

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

Looks good, I'll leave the final approval to @ichard26 so it doesn't auto-merge.
(I think that is how this works)

@GDWR GDWR dismissed their stale review July 11, 2022 09:57

Leaving final approval to ichard26

Copy link
Copy Markdown
Contributor

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

I wasn't able to find time to do a full review with manual testing (and I feel bad about delaying the PR even more), so consider this just a read-through review. On that note, it looks good except for a poorly written comment.

Comment thread bot/exts/core/help.py Outdated
Co-authored-by: Richard Si <63936253+ichard26@users.noreply.github.com>
@RohanJnr RohanJnr requested a review from ichard26 July 14, 2022 08:21
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Had a quick look over, looks good and seems to work nicely, thanks 👍

@Xithrius Xithrius merged commit 4e9aa1e into main Aug 18, 2022
@Xithrius Xithrius deleted the help_command_fix_863 branch August 18, 2022 16:11
@TizzySaurus TizzySaurus mentioned this pull request Sep 7, 2022
4 tasks
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities category: utilities Related to utilities type: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants