Conversation
cli/slsa-verifier/verify/options.go
Outdated
|
|
||
| cmd.Flags().StringVar(&o.AttestationPath, "attestation-path", "", | ||
| "path to a file containing the attestation") | ||
| cmd.MarkFlagFilename("attestation-path", "intoto.jsonl") |
There was a problem hiding this comment.
The filename can actually have any extension.
There was a problem hiding this comment.
Ok, removed this filter.
There was a problem hiding this comment.
Thanks, also remove it for the other subcommands.
There was a problem hiding this comment.
I see we can actually specify multiple extensions, so how about using the common ones: .sigstore, .intoto, .intoto.jsonl, .json.
There was a problem hiding this comment.
Done for both related flags. Thanks for the guidance, I'm familiar with cobra but not that much with all things SLSA.
I suppose it should be noted that at least with bash (well readline more accurately), the user can invoke completion of all filenames by hitting Meta-/ (typically Esc-/) instead of Tab. Even though that's a bit inconvenient and not too well known combo, at least there is an escape hatch if we get some of the file extension filters wrong.
| func (o *VerifyVSAOptions) AddFlags(cmd *cobra.Command) { | ||
| cmd.Flags().StringArrayVar(&o.SubjectDigests, "subject-digest", []string{}, | ||
| "the digests to be verified. Pass multiple digests by repeating the flag. e.g. --subject-digest <digest type>:<digest value> --subject-digest <digest type>:<digest value>") | ||
| cmd.RegisterFlagCompletionFunc("subject-digest", cobra.NoFileCompletions) |
There was a problem hiding this comment.
If we don't add this line, would the behavior be the same?
There was a problem hiding this comment.
No; without it, all filenames in the current dir will be suggested as arguments for the flag.
scop
force-pushed
the
fix/shell-completions
branch
2 times, most recently
from
589b2f3 to
4134a9b
Compare
| if len(args) != 0 { | ||
| return nil, cobra.ShellCompDirectiveNoFileComp | ||
| } |
There was a problem hiding this comment.
This works like a shortcut to say that all of options shouldn't offer completion?
There was a problem hiding this comment.
It means that if we already have one or more positional argument, do not offer any more as completions, as verify-npm-package takes only one positional argument.
scop
force-pushed
the
fix/shell-completions
branch
from
4134a9b to
7aedf88
Compare
cli/slsa-verifier/verify/options.go
Outdated
|
|
||
| var _ Interface = (*VerifyOptions)(nil) | ||
|
|
||
| var CommonFilenameExtensions = []string{"sigstore", "intoto", "intoto.jsonl", "json"} |
There was a problem hiding this comment.
| var CommonFilenameExtensions = []string{"sigstore", "intoto", "intoto.jsonl", "json"} | |
| var CommonAttestationFilenameExtensions = []string{"sigstore", "intoto", "intoto.jsonl", "json"} |
There was a problem hiding this comment.
This is also being used for provenance filenames per earlier comments. Do we still want to rename it as suggested?
There was a problem hiding this comment.
Yes, provenance and attestation are mostly interchangeable terms, but attestation is more generic.
scop
force-pushed
the
fix/shell-completions
branch
from
7aedf88 to
8c2605e
Compare
Done. This would be useful info to have in docs/CONTRIBUTING.md |
Subject digests are not given as positional arguments, but with --subject-digest flags. Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
scop
force-pushed
the
fix/shell-completions
branch
from
8c2605e to
d7880e2
Compare
|
Hi @ramonpetgrave64, others, thank you for the review and approval. I'm wondering if this is pending something still from me or otherwise, or if it could be merged before it could grow conflicts or become stale some other way? |
2 participants
See individual commits and their messages for details.