-
Notifications
You must be signed in to change notification settings - Fork 597
Feat/file flag completion improvements #4028
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
Conversation
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.76% -3.35%
==========================================
Files 155 210 +55
Lines 10044 13386 +3342
==========================================
+ Hits 4028 4921 +893
- Misses 5530 7847 +2317
- Partials 486 618 +132 ☔ 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?
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.
This is discussed in detail in the b6ac9b3 commit message.
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.
But TL;DR: yes :)
cmd/cosign/cli/options/attach.go
Outdated
@@ -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
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.
Done.
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]>
aa5537c
to
b6ac9b3
Compare
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!
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [cosign](https://github.com/sigstore/cosign) | patch | `2.4.2` -> `2.4.3` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>sigstore/cosign (cosign)</summary> ### [`v2.4.3`](https://github.com/sigstore/cosign/blob/HEAD/CHANGELOG.md#v243) [Compare Source](sigstore/cosign@v2.4.2...v2.4.3) #### Features - Bump sigstore/sigstore to support KMS plugins ([#​4073](sigstore/cosign#4073)) - Enable fetching signatures without remote get. ([#​4047](sigstore/cosign#4047)) - Feat/file flag completion improvements ([#​4028](sigstore/cosign#4028)) - Update builder to use go1.23.6 ([#​4052](sigstore/cosign#4052)) #### Bug Fixes - fix parsing error in --only for cosign copy ([#​4049](sigstore/cosign#4049)) #### Cleanup - Refactor verifyNewBundle into library function ([#​4013](sigstore/cosign#4013)) - fix comment typo and imports order ([#​4061](sigstore/cosign#4061)) - sync comment with parameter name in function signature ([#​4063](sigstore/cosign#4063)) - sort properly Go imports ([#​4071](sigstore/cosign#4071)) #### Contributors - Bob Callaway - Carlos Tadeu Panato Junior - Cody Soyland - Dmitry Savintsev - Hayden B - Tomasz Janiszewski - Ville Skyttä </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODAuMiIsInVwZGF0ZWRJblZlciI6IjM5LjE4MC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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