mvdan.cc / cue
this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

cmd/cue: fix regression when injecting vars with cue cmd

The -t and -T flags can be used like:

cue cmd -t foo=var -T mycmd .

However, we also support a short form, where "cmd" can be omitted:

cue -t foo=var -T mycmd .

After the refactor in https://cuelang.org/cl/552733,
we made cmd/cue more principled in how it handles the root command,
and we broke those -t and -T flags in the "cue cmd" short form.
We didn't intend to - they just started being treated as global flags.

A simple and easy fix is to actually declare these flags globally,
and not just as part of the "cue cmd" sub-command.
Since each of these flags now exists in two levels,
the only place where they are consumed, the setTags function,
now checks both flag sets and joins their values.

Note that we make the new global flags hidden,
so that "cue help" doesn't start showing them as global flags.
We don't really intend them as global flags for every command.
We also avoid adding them to PersistentFlags,
so that they are not inherited by sub-commands like fmt.

This fix is preferrable to an entire revert of the previous CL,
as otherwise commands like "cue help" would become very slow again.

I've also filed #2519 to propose deprecating the "cue cmd" short form.

Fixes #2510.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: I254e1c4e8ef4d6ffa7d6142c75abee5f1a37a810
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/557127
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>

Daniel Martí da3ee7dc 43b46748

+42 -13
+1 -1
cmd/cue/cmd/cmd.go
··· 181 181 182 182 cmd.Flags().SetInterspersed(false) 183 183 184 - addInjectionFlags(cmd.Flags(), true) 184 + addInjectionFlags(cmd.Flags(), true, false) 185 185 186 186 return cmd 187 187 }
+12 -7
cmd/cue/cmd/common.go
··· 400 400 } 401 401 cfg.reFile = re 402 402 403 - if err := setTags(cmd.Flags(), cfg.loadCfg); err != nil { 403 + if err := setTags(cfg.loadCfg, cmd.Flags(), cmd.Parent().Flags()); err != nil { 404 404 return nil, err 405 405 } 406 406 ··· 411 411 return p.cfg.reFile.MatchString(file) 412 412 } 413 413 414 - func setTags(f *pflag.FlagSet, cfg *load.Config) error { 415 - tags, _ := f.GetStringArray(string(flagInject)) 416 - cfg.Tags = tags 417 - if b, _ := f.GetBool(string(flagInjectVars)); b { 418 - cfg.TagVars = load.DefaultTagVars() 414 + func setTags(cfg *load.Config, flags ...*pflag.FlagSet) error { 415 + // setTags usually receives a subcommmand's flags, plus the parent (root) command's flags, 416 + // so that we can support "cue cmd -t foo=bar mycmd" as well as the short form "cue -t foo=bar mycmd". 417 + // TODO: once we remove "cue cmd" short-hand support, go back to a single FlagSet parameter. 418 + for _, f := range flags { 419 + tags, _ := f.GetStringArray(string(flagInject)) 420 + cfg.Tags = append(cfg.Tags, tags...) 421 + if b, _ := f.GetBool(string(flagInjectVars)); b { 422 + cfg.TagVars = load.DefaultTagVars() 423 + } 419 424 } 420 425 return nil 421 426 } ··· 769 774 Tools: true, 770 775 } 771 776 f := cmd.cmd.Flags() 772 - if err := setTags(f, cfg); err != nil { 777 + if err := setTags(cfg, f, cmd.cmd.Parent().Flags()); err != nil { 773 778 return nil, err 774 779 } 775 780
+1 -1
cmd/cue/cmd/def.go
··· 37 37 38 38 addOutFlags(cmd.Flags(), true) 39 39 addOrphanFlags(cmd.Flags()) 40 - addInjectionFlags(cmd.Flags(), false) 40 + addInjectionFlags(cmd.Flags(), false, false) 41 41 42 42 cmd.Flags().StringArrayP(string(flagExpression), "e", nil, "evaluate this expression only") 43 43
+1 -1
cmd/cue/cmd/eval.go
··· 54 54 55 55 addOutFlags(cmd.Flags(), true) 56 56 addOrphanFlags(cmd.Flags()) 57 - addInjectionFlags(cmd.Flags(), false) 57 + addInjectionFlags(cmd.Flags(), false, false) 58 58 59 59 cmd.Flags().StringArrayP(string(flagExpression), "e", nil, "evaluate this expression only") 60 60
+1 -1
cmd/cue/cmd/export.go
··· 94 94 95 95 addOutFlags(cmd.Flags(), true) 96 96 addOrphanFlags(cmd.Flags()) 97 - addInjectionFlags(cmd.Flags(), false) 97 + addInjectionFlags(cmd.Flags(), false, false) 98 98 99 99 cmd.Flags().Bool(string(flagEscape), false, "use HTML escaping") 100 100 cmd.Flags().StringArrayP(string(flagExpression), "e", nil, "export this expression only")
+5 -1
cmd/cue/cmd/flags.go
··· 88 88 f.Bool(string(flagMerge), true, "merge non-CUE files") 89 89 } 90 90 91 - func addInjectionFlags(f *pflag.FlagSet, auto bool) { 91 + func addInjectionFlags(f *pflag.FlagSet, auto, hidden bool) { 92 92 f.StringArrayP(string(flagInject), "t", nil, 93 93 "set the value of a tagged field") 94 94 f.BoolP(string(flagInjectVars), "T", auto, 95 95 "inject system variables in tags") 96 + if hidden { 97 + f.Lookup(string(flagInject)).Hidden = true 98 + f.Lookup(string(flagInjectVars)).Hidden = true 99 + } 96 100 } 97 101 98 102 type flagName string
+3
cmd/cue/cmd/root.go
··· 191 191 subCommands = append(subCommands, newHelpTopics(c)...) 192 192 193 193 addGlobalFlags(cmd.PersistentFlags()) 194 + // We add the injection flags to the root command for the sake of the short form "cue -t foo=bar mycmd". 195 + // Note that they are not persistent, so that they aren't inherited by sub-commands like "cue fmt". 196 + addInjectionFlags(cmd.Flags(), false, true) 194 197 195 198 for _, sub := range subCommands { 196 199 cmd.AddCommand(sub)
+17
cmd/cue/cmd/testdata/script/cmd_tags.txtar
··· 1 1 exec cue cmd -t prod -t name=bar tag tags.cue tags_tool.cue 2 2 cmp stdout expect-stdout 3 3 4 + # Same as before, but now with the short form. 5 + # See https://cuelang.org/issue/2510. 6 + exec cue -t prod -t name=bar tag tags.cue tags_tool.cue 7 + cmp stdout expect-stdout 8 + 9 + # Verify that the new global -t flag added as a fix for issue 2510 above 10 + # works with the explicit or implicit "cmd" sub-command, 11 + # but not with other sub-commands like "fmt". 12 + exec cue eval -t name=bar tags.cue 13 + stdout 'name: *"bar"' 14 + exec cue -t name=bar eval tags.cue 15 + stdout 'name: *"bar"' 16 + ! exec cue fmt -t name=bar tags.cue 17 + stderr 'unknown shorthand flag' 18 + ! exec cue -t name=bar fmt tags.cue 19 + stderr 'unknown shorthand flag' 20 + 4 21 -- expect-stdout -- 5 22 prod: bar 6 23 -- tags.cue --
+1 -1
cmd/cue/cmd/vet.go
··· 70 70 } 71 71 72 72 addOrphanFlags(cmd.Flags()) 73 - addInjectionFlags(cmd.Flags(), false) 73 + addInjectionFlags(cmd.Flags(), false, false) 74 74 75 75 cmd.Flags().BoolP(string(flagConcrete), "c", false, 76 76 "require the evaluation to be concrete")