-
Notifications
You must be signed in to change notification settings - Fork 775
Update bash completions for sonic-utilities commands #4163
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
Update the bash completion files for all sonic-utilities commands to make them compatible with the current Click version. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Pull request overview
This PR modernizes bash completion scripts for sonic-utilities commands to be compatible with the current version of Click. The changes migrate from Click's deprecated completion format (_COMPLETE=complete) to the modern format (_COMPLETE=bash_complete), which properly handles different completion types (directories, files, and plain text).
Key Changes:
- Updated all bash completion files to use Click's new
bash_completecompletion API - Standardized all completion scripts to a consistent 29-line template
- Fixed typos in rshell and rexec completion setup function names
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| watermarkstat | New completion file following the standardized template |
| watchdogutil | New completion file following the standardized template |
| undebug | Updated from old Click completion format to new bash_complete format |
| switchstat | New completion file following the standardized template |
| spm | Replaced reference to sonic-package-manager with full completion implementation |
| sonic_installer | Updated from old format to new bash_complete format |
| sonic-package-manager | Updated from old format to new bash_complete format |
| sonic-installer | Updated from old format to new bash_complete format |
| sonic-cli-gen | Updated from old format to new bash_complete format |
| sonic-clear | Updated from old format to new bash_complete format, but removed 'clear' alias |
| sonic-bootchart | New completion file following the standardized template |
| show | Updated from old format to new bash_complete format |
| sfputil | Updated from old format to new bash_complete format |
| sfpshow | New completion file following the standardized template |
| rshell | Updated from old format, fixed typo in function name, removed bash version check |
| rexec | Updated from old format, fixed typo in function name, removed bash version check |
| queuestat | New completion file following the standardized template |
| psuutil | Updated from old format to new bash_complete format |
| pfcwd | Updated from old format to new bash_complete format |
| pfc | Updated from old format to new bash_complete format |
| pddf_thermalutil | Updated from old format to new bash_complete format |
| pddf_psuutil | Updated from old format to new bash_complete format |
| pddf_ledutil | Updated from old format to new bash_complete format |
| pddf_fanutil | Updated from old format to new bash_complete format |
| pcieutil | New completion file following the standardized template |
| null_route_helper | New completion file following the standardized template |
| mmuconfig | New completion file following the standardized template |
| fwutil | Entire file deleted - completion removed for fwutil command |
| ecnconfig | New completion file following the standardized template |
| dump | Updated from old format to new bash_complete format |
| dropstat | New completion file following the standardized template |
| debug | Updated from old format to new bash_complete format |
| crm | Updated from old format to new bash_complete format |
| counterpoll | Updated from old format to new bash_complete format |
| consutil | Updated from old format to new bash_complete format |
| connect | Updated from old format to new bash_complete format |
| config | Updated from old format to new bash_complete format |
| buffershow | New completion file following the standardized template |
| acl-loader | Updated from old format to new bash_complete format, changed return code from 1 to 0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| complete -F _sonic_clear_completion -o default sonic-clear; | ||
| complete -F _sonic_clear_completion clear; | ||
| _sonic_clear_completion_setup() { | ||
| complete -o nosort -F _sonic_clear_completion sonic-clear |
Copilot
AI
Dec 18, 2025
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.
The old completion file included a second complete command for the 'clear' alias: complete -F _sonic_clear_completion clear;. This line should be restored to maintain backward compatibility with the 'clear' command alias.
| complete -o nosort -F _sonic_clear_completion sonic-clear | |
| complete -o nosort -F _sonic_clear_completion sonic-clear | |
| complete -F _sonic_clear_completion clear; |
| fi | ||
| done | ||
|
|
||
| return 0 |
Copilot
AI
Dec 18, 2025
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.
The old completion function returned 1, but the new implementation returns 0. While returning 0 is more standard for bash completion functions, verify that this change doesn't break any expected behavior, as the original non-zero return may have been intentional.
| return 0 | |
| return 1 |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Is it possible to automate to generate the bash completion scripts? |
anders-nexthop
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.
Looks okay to me but I agree with @qiluo-msft that having a way to generate these would be desirable. It would be pretty easy to miss adding the completion script for new commands.
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
Update the show and watchdog CLIs so that the modules are at least importable outside of SONiC so that completion code can be generated. Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Saikrishna Arcot <sarcot@microsoft.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
What I did
Update the bash completion files for all sonic-utilities commands to make them compatible with the current Click version.
Fixes sonic-net/sonic-buildimage#24594.
How I did it
Use Click's documentation to generate the bash completion script for each command that is packaged from sonic-utilities and uses Click.
How to verify it
Tested in KVM in Trixie image.
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)