search

cuelang / cue

CUE has moved to https://github.com/cue-lang/cue
https://cuelang.org
Apache License 2.0
3.09k stars 171 forks source link

cmd/cue: import should ignore null documents #1039

Closed uhthomas closed 3 years ago

uhthomas commented 3 years ago

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"
}]
uhthomas commented 3 years ago

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.

uhthomas commented 3 years ago

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]
myitcv commented 3 years ago

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.

cueckoo commented 3 years ago

This issue has been migrated to https://github.com/cue-lang/cue/issues/1039.

For more details about CUE's migration to a new home, please see https://github.com/cue-lang/cue/issues/1078.