-
Notifications
You must be signed in to change notification settings - Fork 38
feat: add bash completion for p11* tools #66
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
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 adds Bash autocompletion support for various p11* CLI tools by introducing a shared common script and individual completion scripts, and updating the Makefile.am to install them.
- Introduce
completion/p11-commonwith reusable parsing and completion functions. - Add dedicated completion scripts for
p11cat,p11cp,p11kcv,p11ls,p11more,p11mv,p11od, andp11setattr. - Update
Makefile.amto include these scripts inEXTRA_DISTand install them underbash-completion/completions.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| completion/p11-common | Shared functions for parsing -l, -s, -p and completing filters |
| completion/p11cat | Autocompletion for p11cat |
| completion/p11cp | Autocompletion for p11cp |
| completion/p11kcv | Autocompletion for p11kcv |
| completion/p11ls | Autocompletion for p11ls |
| completion/p11more | Autocompletion for p11more |
| completion/p11mv | Autocompletion for p11mv |
| completion/p11od | Autocompletion for p11od |
| completion/p11setattr | Autocompletion for p11setattr |
| Makefile.am | Registers and installs completion scripts |
Comments suppressed due to low confidence (2)
completion/p11mv:3
- The comment refers to
p11cpbut this is thep11mvcompletion script; update it top11mv.
# Bash autocompletion for `p11cp`.
completion/p11-common:60
- Consider adding unit or integration tests for
__p11_complete_filtersto ensure completion logic works correctly across different argument combinations.
__p11_complete_filters() {
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
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.
It would be great to have support for slot selection. That would require to modify p11slotinfo and add an option to enumerate slots and token, for completion.
keldonin
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.
Good stuff.
A few requests:
- It should support ZSH as well
- If possible, infer slots as well - that would require to adapt
p11slotinfoto yield a list of slots and token, for completion.
This PR adds bash completion support for several
p11*CLI tools.Features
completion/p11-commonp11cat,p11cp,p11kcv,p11ls,p11more,p11mv,p11od,p11setattrPKCS11LIB,PKCS11SLOT,PKCS11PASSWORDas environment fallbackMakefile
EXTRA_DIST${datadir}/bash-completion/completionsLimitations
PKCS11LIB=... p11cat)