-
-
Notifications
You must be signed in to change notification settings - Fork 839
✨ Enhance command alias handling and display #1422
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
base: master
Are you sure you want to change the base?
Conversation
- Added support for positional and additional aliases in the command decorator. - Updated `format_commands` to display command names along with their visible aliases. - Improved command listing to maintain original order and avoid duplicates. - Enhanced command info structure to include aliases and hidden aliases. This improves the usability and clarity of command help output.
…r into feature/CommandAliases fix some type errors in github actions
added document for new changes
📝 Docs previewLast commit b91a994 at: https://e450f7c8.typertiangolo.pages.dev Modified Pages |
This enhances the test coverage for command alias functionality.
…r into feature/CommandAliases
Enhanced coverage for command help output to verify alias presence and hidden status.
svlandeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pipinstalled: you can go ahead and set add # pragma: no cover to the pass statements in the tests, otherwise the coverage check will keep failing.
@svlandeg I added # pragma but still coverage failed |
You'll have to look into the specifics of the coverage report. What this typically means is that there's a code path that is untested, so you'd have to implement more tests to cover it. |
… output is disabled
…r into feature/CommandAliases
…ch output is disabled
|
@svlandeg Thanks for your help, fixed with adding new tests. |
|
@svlandeg Have you any idea when this PR will merge? :D |
This comment was marked as resolved.
This comment was marked as resolved.
…cy with formatting
…r into feature/CommandAliases
|
@tiangolo Do you have any idea when this PR will merge? :) |
|
This is great! Right now I am hacking around it (basnijholt/compose-farm#148). |
|
This will be great, I went the route of creating a custom alias class but I recently realised it broke shell completion :/ Thanks for all the efforts you've gone to with this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, @pipinstalled!
I can see how it can be useful to define an alias for a given command.
In this specific PR, there are two ways implemented to accomplish that: either by positional arguments, or using an aliases= parameter. I'm not usually keen on implementing different ways of accomplishing the same thing: it creates confusion for users and unnecessary code maintenance.
Further, there is already a way (on master) to register different names for the same command:
@app.command("list")
@app.command("ls")
@app.command("listthem", hidden=True)
def list_items():
print("Listing items")
I know that that would show up slightly differently in the help, they would show up as separate commands. At the same time I do also worry that this PR adds a new format_commands function and edits the Rich formatting in _print_commands_panel - it feels like these are potential cases for regressions or future issues.
TLDR: I'm not convinced that this a feature we'll want to support, especially considering the amount of changes needed in core etc. I'll run this by Tiangolo though, to see what he thinks.
| assert "list" in result.output or "ls" in result.output | ||
| assert ( | ||
| "remove" in result.output or "rm" in result.output or "delete" in result.output | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should all be and instead of or, right?
format_commandsto display command names along with their visible aliases.This improves the usability and clarity of command help output.