Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
cmd/cue/cmd: ignore optional fields for trim
Browse files Browse the repository at this point in the history
There are rare cases where trim might remove an
optional field, but optional fields are more likely
to interfere with the removal logic.

Change-Id: Iab5b680cee9dbf0e0f4b06979ef43d2f0a5c6001
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/1804
Reviewed-by: Marcel van Lohuizen <[email protected]>
  • Loading branch information
mpvl committed Apr 19, 2019
1 parent 2b85fc8 commit e9fd214
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 1 deletion.
12 changes: 11 additions & 1 deletion cmd/cue/cmd/trim.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ var (
)

func runTrim(cmd *cobra.Command, args []string) error {
// TODO: Do something more fine-grained. Optional fields are mostly not
// useful to consider as an optional field will almost never subsume
// another value. However, an optional field may subsume and therefore
// trigger the removal of another optional field.
// For now this is the better approach: trimming is not 100% accurate,
// and optional fields are just more likely to cause edge cases that may
// block a removal.
internal.DropOptional = true
defer func() { internal.DropOptional = false }()

log.SetOutput(cmd.OutOrStderr())

ctxt := build.NewContext(build.ParseOptions(parser.ParseComments))
Expand Down Expand Up @@ -352,7 +362,7 @@ func (t *trimSet) trim(label string, v, m, scope cue.Value) (rmSet []ast.Node) {

// Build map of mixin fields.
valueMap := map[key]cue.Value{}
for mIter, _ := in.Fields(cue.All()); mIter.Next(); {
for mIter, _ := in.Fields(cue.All(), cue.Optional(false)); mIter.Next(); {
valueMap[iterKey(mIter)] = mIter.Value()
}

Expand Down
4 changes: 4 additions & 0 deletions cue/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"cuelang.org/go/cue/build"
"cuelang.org/go/cue/literal"
"cuelang.org/go/cue/token"
"cuelang.org/go/internal"
)

// insertFile inserts the given file at the root of the instance.
Expand Down Expand Up @@ -285,6 +286,9 @@ func (v *astVisitor) walk(astNode ast.Node) (value value) {
}

case *ast.BasicLit, *ast.Ident:
if internal.DropOptional && opt {
break
}
attrs, err := createAttrs(v.ctx(), newNode(n), n.Attrs)
if err != nil {
return err
Expand Down
4 changes: 4 additions & 0 deletions internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,7 @@ var FromGoValue func(instance, x interface{}) interface{}
// instance must be of type *cue.Instance.
// The returned value is a cue.Value, which the caller must cast to.
var FromGoType func(instance, x interface{}) interface{}

// DropOptional is a blanket override of handling optional values during
// compilation. TODO: should we make this a build option?
var DropOptional bool

0 comments on commit e9fd214

Please sign in to comment.