-
Notifications
You must be signed in to change notification settings - Fork 557
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
Feat/file flag completion improvements #4028
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Ville Skyttä <[email protected]>
Be more consistent across with extensions accepted/filtered, add some. Also, mark and comment out cases where there are no known typical filename extensions for flags taking filename arguments, to make it obvious that they have not been inadvertently omitted. Marking a flag as filename without specifying extensions is a no-op, and actually considered a bug per commentary in cobra sources: https://github.com/spf13/cobra/blob/41b26ec8bb59dfba580f722201bf371c4f5703dd/completions.go#L387-L390 Closes sigstore/community#538 Signed-off-by: Ville Skyttä <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4028 +/- ##
==========================================
- Coverage 40.10% 36.42% -3.68%
==========================================
Files 155 209 +54
Lines 10044 13377 +3333
==========================================
+ Hits 4028 4873 +845
- Misses 5530 7883 +2353
- Partials 486 621 +135 ☔ View full report in Codecov by Sentry. |
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.
Thank you! Just one question
@@ -107,5 +108,5 @@ func (o *AttestBlobOptions) AddFlags(cmd *cobra.Command) { | |||
|
|||
cmd.Flags().StringVar(&o.RFC3161TimestampPath, "rfc3161-timestamp-bundle", "", | |||
"path to an RFC 3161 timestamp bundle FILE") | |||
_ = cmd.Flags().SetAnnotation("rfc3161-timestamp-bundle", cobra.BashCompFilenameExt, []string{}) | |||
// _ = cmd.MarkFlagFilename("rfc3161-timestamp-bundle") // no typical extensions |
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.
To confirm, is there any purpose to including this? As in, will shell completion still work with rfc3161-...
for example?
@@ -80,7 +80,7 @@ func (o *AttachSBOMOptions) AddFlags(cmd *cobra.Command) { | |||
|
|||
cmd.Flags().StringVar(&o.SBOM, "sbom", "", | |||
"path to the sbom, or {-} for stdin") | |||
_ = cmd.Flags().SetAnnotation("sbom", cobra.BashCompFilenameExt, []string{}) | |||
cmd.MarkFlagFilename("sbom", sbomExts...) |
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.
fix lint here
Summary
As described in sigstore/community#538
To test, generate and install shell completions and invoke completion for flags taking filenames as arguments, observe results.
Release Note
NONE
Documentation
NONE