Skip to content
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

cue/format: most options are not respected #2404

Open
verdverm opened this issue May 13, 2023 · 4 comments
Open

cue/format: most options are not respected #2404

verdverm opened this issue May 13, 2023 · 4 comments

Comments

@verdverm
Copy link

verdverm commented May 13, 2023

What version of CUE are you using (cue version)?

0.5.0

Does this issue reproduce with the latest stable release?

No, but it worked as expected in 0.4.3

What did you do?

package main

import (
	"fmt"

	"cuelang.org/go/cue"
	"cuelang.org/go/cue/cuecontext"
	"cuelang.org/go/cue/format"
)

var input = `
// a doc string
obj: {
	@hello(world)
	a: int | *1
	v: bool | *false
}
opt?: "optional"
#def: "definition"
_hid: "hidden"
`

func main() {

	ctx := cuecontext.New()
	val := ctx.CompileString(input)	

	s, _ := str(val)

	fmt.Println(s)
}

func str(val cue.Value) (string, error) {
	opts := []cue.Option{
		cue.Optional(false),
		cue.Definitions(false),
		cue.Hidden(false),
		cue.Attributes(false),     // <==== only option respected
		cue.Docs(false),
	}

	syn := val.Syntax(opts...)

	bs, err := format.Node(syn)
	if err != nil {
		return "", err
	}

	return string(bs), nil
}

What did you expect to see?

{
        obj: {
                a: int | *1
                v: bool | *false
        }
}

What did you see instead?\

{
        // a doc string
        obj: {
                a: int | *1
                v: bool | *false
        }
        opt?: "optional"
        #def: "definition"
        _hid: "hidden"
}

Output from v0.4.3

$ go run main.go 
{
        // a doc string
        obj: {
                a: int | *1
                v: bool | *false
        }
        opt?: "optional"
        #def: "definition"
        _hid: "hidden"
}
@verdverm verdverm added NeedsInvestigation Triage Requires triage/attention labels May 13, 2023
@myitcv myitcv added the zGarden label Jun 13, 2023
@mpvl mpvl added the FeatureRequest New feature or request label Jun 13, 2023
@mpvl mpvl removed the Triage Requires triage/attention label Jun 13, 2023
@mvdan mvdan added the FeatureRequest New feature or request label Jun 13, 2023
@myitcv
Copy link
Member

myitcv commented Jun 13, 2023

The docs almost certainly need improving here, hence the Documentation label. But we also need to reshape the API here, hence the roadmap/api label because it's not at all clear/"useful".

@verdverm
Copy link
Author

verdverm commented Jun 21, 2023

@myitcv this was working before 0.5.0, so this is something that broke existing code we've had for many versions.

I think bug is an appropriate label here

(added output from 0.4.3 to original description)


edit, I may have my testing matrix output confused... will update

what is curious is that

  • the flags for cue respect the settings
  • it looks like the cue cmd is using the same underlying API (so far as I can tell right now)
    So why does it work in one place but not the other?

@verdverm
Copy link
Author

verdverm commented Jun 21, 2023

Upon further investigation, the behavior looks to be stable.

What does make a difference is if you use a cue.Final() in the options, then more of the other options are respected. This is what the cue eval command is doing, always applying final (https://github.com/cue-lang/cue/blob/v0.5.0/cmd/cue/cmd/eval.go#L92). Thus there is no way to print a value an preserve a preference mark, without also getting all of the hidden and definition fields.

I generally think the documentation makes sense as is for what the expected behavior ought to be, and that the implementation probably needs more conditions to cover the non-final path. There are some other curiosities, like setting hidden implies the definition setting is the same (https://github.com/cue-lang/cue/blob/v0.5.0/cue/types.go#L2181)

@verdverm
Copy link
Author

verdverm commented Jun 21, 2023

Another important aspect is that the order of options applied also matters, and later options can override earlier options.

Underneath, these Option functions set values in a struct. I think a better API would expose the options as a struct, so the user can set them as appropriate and avoid the opaque ordering issue in the current API. It would be much more declarative than imperative, i.e. order shouldn't matter, like CUE :]


final first

opts := []cue.Option{
	cue.Final(),
	cue.Optional(true),
	cue.Definitions(false),
	cue.Hidden(true),
	cue.Attributes(false),
	cue.Docs(false),
}

---

$ go run main.go
{
        // a doc string
        obj: {
                a: 1
                v: false
        }
        opt?: "optional"
        #def: "definition"
        _hid: "hidden"
}

final last

opts := []cue.Option{
	cue.Optional(true),
	cue.Definitions(false),
	cue.Hidden(true),
	cue.Attributes(false),
	cue.Docs(false),
	cue.Final(),
}

---
$ go run main.go
{
        // a doc string
        obj: {
                a: 1
                v: false
        }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants