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

cmd/cue: import should ignore null documents #1039

Open
cueckoo opened this issue Jul 3, 2021 · 3 comments
Open

cmd/cue: import should ignore null documents #1039

cueckoo opened this issue Jul 3, 2021 · 3 comments
Labels
Discuss Requires maintainer discussion FeatureRequest New feature or request

Comments

@cueckoo
Copy link
Collaborator

cueckoo commented Jul 3, 2021

Originally opened by @uhthomas in cuelang/cue#1039

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

$ cue version
cue version v0.4.0 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What did you do?

Run cue import --list test.yaml against a YAML file with null documents:

---
some_key: some value

---

---
some_other_key: some other value

What did you expect to see?

Two CUE objects.

What did you see instead?

Three CUE objects, one being null.

[{
        some_key: "some value"
}, null, {
        some_other_key: "some other value"
}]
@cueckoo cueckoo added NeedsInvestigation Triage Requires triage/attention labels Jul 3, 2021
@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @uhthomas in cuelang/cue#1039 (comment)

I hit this quite a lot when importing Kubernetes generated manifests, like Reloader's manifest.

The output ends up being lots of null, null, null.

Not really sure if this is a bug or a feature request as I can imagine scenarios where this behavior could be desired.

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @uhthomas in cuelang/cue#1039 (comment)

Not sure if this should be a separate issue -- the imported CUE definitions get really weird with comments.

This is the output for the example manifest I linked earlier:

// <snip>
}, null, null, null, {
	// Source: reloader/templates/role.yaml

	// Source: reloader/templates/rolebinding.yaml

	// Source: reloader/templates/service.yaml

	// Source: reloader/templates/serviceaccount.yaml

	apiVersion: "v1"
// </snip>

All of the comments from the "null" documents get shoved into the proceeding object.

Interestingly, if there is no proceeding object, these comments are just dropped entirely.

---
some_key: some value

---

# some comment

---

# some other comment

---
[{
        some_key: "some value"
}, null, null, null]

@cueckoo
Copy link
Collaborator Author

cueckoo commented Jul 3, 2021

Original reply by @myitcv in cuelang/cue#1039 (comment)

Not really sure if this is a bug or a feature request as I can imagine scenarios where this behavior could be desired.

It's not clear to me from the Yaml spec that what CUE does here is wrong, indeed quite the opposite:

https://yaml.org/spec/1.2/spec.html#id2786563

So I would instead consider this a feature request. With the query syntax (#165) something like this could be quite natural/easy to represent too.

Not sure if this should be a separate issue -- the imported CUE definitions get really weird with comments.

Comments should indeed be a separate issue (I understand from @mpvl comments are a "challenge" when it comes to Yaml).

Marking as "Triage" for discussion with @mpvl in any case.

@myitcv myitcv added Discuss Requires maintainer discussion and removed Triage Requires triage/attention labels Jul 29, 2021
@mpvl mpvl added FeatureRequest New feature or request and removed NeedsInvestigation labels Nov 24, 2021
@myitcv myitcv added zzz Discuss and removed Discuss Requires maintainer discussion labels Jan 29, 2022
@myitcv myitcv added Discuss Requires maintainer discussion and removed zzz Discuss labels Feb 9, 2022
@myitcv myitcv added the zGarden label Jun 15, 2023
@mvdan mvdan removed the zGarden label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discuss Requires maintainer discussion FeatureRequest New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants