-
Notifications
You must be signed in to change notification settings - Fork 249
Adds std.parseYaml #339
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
Merged
Merged
Adds std.parseYaml #339
Changes from all commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
f4d034e
Init parseYaml.
groodt 21e3b5c
Tabs.
groodt fc0c0b0
Use kubernetes-sigs/yaml
groodt 630dc4a
Use kubernetes-sigs/yaml
groodt a2db0ae
Use kubernetes-sigs/yaml
groodt 47ade21
yaml.go
groodt ce7efb2
Merge branch 'master' into groodt-parseYaml
groodt 973d881
Updated modules.
groodt e5872ce
Merge branch 'master' into groodt-parseYaml
groodt 4a5cffe
Fix missed conflicts.
groodt aacec92
Fix missed conflicts.
groodt 2384297
Fix missed conflicts.
groodt 73e417e
Fix missed conflicts.
groodt 5648104
Fix missed conflicts.
groodt a5ea27a
Merge branch 'master' into groodt-parseYaml
groodt 561f9a1
Fix go compilation.
groodt 713877e
CPP parity.
groodt f2a5e25
go fmt
groodt 87616db
golint
groodt aa08bfb
go mod tidy
groodt 482e950
Update submodules
groodt 76fd830
Remove EOL Go 1.11 and 1.12 from CI
groodt 40787e1
Revert submodule change back to upstream
sbarzowski 5ae48fb
Merge branch 'master' into groodt-parseYaml
sbarzowski 8c1f0d7
Fix the bad merge
sbarzowski e109647
Deps fix
sbarzowski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,8 +3,6 @@ sudo: false | |
| matrix: | ||
| include: | ||
| - go: 1.x | ||
| - go: 1.11.x | ||
| - go: 1.12.x | ||
| - go: 1.13.x | ||
| - go: 1.x | ||
| arch: amd64 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Submodule cpp-jsonnet
updated
308 files
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "aaa": { }, | ||
| "foo": "bar", | ||
| "xxx": [ | ||
| 42, | ||
| "asdf", | ||
| { } | ||
| ], | ||
| "ąę": "ćż" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| std.parseYaml( | ||
| ||| | ||
| foo: bar | ||
| aaa: {} | ||
| ąę: ćż | ||
| xxx: | ||
| - 42 | ||
| - asdf | ||
| - {} | ||
| ||| | ||
| ) |
Empty file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| /* | ||
| Copyright 2019 Google Inc. All rights reserved. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package jsonnet | ||
|
|
||
| import ( | ||
| "bufio" | ||
| "bytes" | ||
| "io" | ||
| "strings" | ||
| "unicode" | ||
|
|
||
| "sigs.k8s.io/yaml" | ||
| ) | ||
|
|
||
| const separator = "---" | ||
|
|
||
| // YAMLToJSONDecoder decodes YAML documents from an io.Reader by | ||
| // separating individual documents. It first converts the YAML | ||
| // body to JSON, then unmarshals the JSON. | ||
| type YAMLToJSONDecoder struct { | ||
| reader Reader | ||
| } | ||
|
|
||
| // NewYAMLToJSONDecoder decodes YAML documents from the provided | ||
| // stream in chunks by converting each document (as defined by | ||
| // the YAML spec) into its own chunk, converting it to JSON via | ||
| // yaml.YAMLToJSON, and then passing it to json.Decoder. | ||
| func NewYAMLToJSONDecoder(r io.Reader) *YAMLToJSONDecoder { | ||
| reader := bufio.NewReader(r) | ||
| return &YAMLToJSONDecoder{ | ||
| reader: NewYAMLReader(reader), | ||
| } | ||
| } | ||
|
|
||
| // Decode reads a YAML document as JSON from the stream or returns | ||
| // an error. The decoding rules match json.Unmarshal, not | ||
| // yaml.Unmarshal. | ||
| func (d *YAMLToJSONDecoder) Decode(into interface{}) error { | ||
| bytes, err := d.reader.Read() | ||
| if err != nil && err != io.EOF { | ||
| return err | ||
| } | ||
|
|
||
| if len(bytes) != 0 { | ||
| err := yaml.Unmarshal(bytes, into) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
| return err | ||
| } | ||
|
|
||
| // Reader reads bytes | ||
| type Reader interface { | ||
| Read() ([]byte, error) | ||
| } | ||
|
|
||
| // YAMLReader reads YAML | ||
| type YAMLReader struct { | ||
| reader Reader | ||
| } | ||
|
|
||
| // NewYAMLReader creates a new YAMLReader | ||
| func NewYAMLReader(r *bufio.Reader) *YAMLReader { | ||
| return &YAMLReader{ | ||
| reader: &LineReader{reader: r}, | ||
| } | ||
| } | ||
|
|
||
| // Read returns a full YAML document. | ||
| func (r *YAMLReader) Read() ([]byte, error) { | ||
| var buffer bytes.Buffer | ||
| for { | ||
| line, err := r.reader.Read() | ||
| if err != nil && err != io.EOF { | ||
| return nil, err | ||
| } | ||
|
|
||
| sep := len([]byte(separator)) | ||
| if i := bytes.Index(line, []byte(separator)); i == 0 { | ||
| // We have a potential document terminator | ||
| i += sep | ||
| after := line[i:] | ||
| if len(strings.TrimRightFunc(string(after), unicode.IsSpace)) == 0 { | ||
| if buffer.Len() != 0 { | ||
| return buffer.Bytes(), nil | ||
| } | ||
| if err == io.EOF { | ||
| return nil, err | ||
| } | ||
| } | ||
| } | ||
| if err == io.EOF { | ||
| if buffer.Len() != 0 { | ||
| // If we're at EOF, we have a final, non-terminated line. Return it. | ||
| return buffer.Bytes(), nil | ||
| } | ||
| return nil, err | ||
| } | ||
| buffer.Write(line) | ||
| } | ||
| } | ||
|
|
||
| // LineReader reads single lines. | ||
| type LineReader struct { | ||
| reader *bufio.Reader | ||
| } | ||
|
|
||
| // Read returns a single line (with '\n' ended) from the underlying reader. | ||
| // An error is returned iff there is an error with the underlying reader. | ||
| func (r *LineReader) Read() ([]byte, error) { | ||
| var ( | ||
| isPrefix bool = true | ||
| err error = nil | ||
| line []byte | ||
| buffer bytes.Buffer | ||
| ) | ||
|
|
||
| for isPrefix && err == nil { | ||
| line, isPrefix, err = r.reader.ReadLine() | ||
| buffer.Write(line) | ||
| } | ||
| buffer.WriteByte('\n') | ||
| return buffer.Bytes(), err | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we can get bitten by some boundary case here. Can it be a part of a multi-line comment? How other implementations handle that?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shotgun parsing is a bit gross, I agree. Anyone adding those characters to a YAML document will be in for a difficult time if they aren't escaping it thoroughly. The
.Containscheck can probably be made more explicit to check for start of line or end of line characters if that makes you feel more comfortable?Alternatively, we can adjust the user facing API. The only reason for "sniffing" whether a document is a "yaml stream" or not is to be perhaps more user-friendly by converting a single YAML document to an object
{}and a stream of YAML documents into an array[]. An alternative user API could be to require a user to choose. e.g.parseYamlthat returns a single object{}andparseYamlStreamwhich returns an array.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think you could achieve the same effect in a cleaner way just by checking
len(elems). The elems will only have multiple elements when there are multiple documents. If it's an array, it will still be just one document. Is that correct?That could work. It's more complicated, but cleaner in a way. I don't have a strong opinion. We can also add parseYamlStream later (which forces returning of an array, even if there is only one document).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we differentiate between a single YAML document with a scalar list vs a YAML stream of documents?
YAML, like JSON, can have a top level sequence or array. Some examples.
Since there can be scalar arrays at the root, checking the
len(elems)isn't enough because it isn't possible to tell the difference between a yaml stream with 1 documentlen(elems) == 1and a single document with a top-level scalar array of length 1.The approach used in the current implementation assumes that if you specify a YAML stream using
---you know you will receive an array of 1 or more documents. If you do not specify a YAML stream, you will get whatever YAML object you specified, either a Map or a Seq of length, 0, 1 or more elements.YAML is complicated! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please correct me if I misunderstand something. I assumed that
d.Decode(&elem)reads one document. So len(elems) will be the number of documents in a stream. If it's only one document (even if it's an array) it would read the whole thing. Is that correct?Is your point that:
and
both produce
{foo: "bar"}, but the user might have expected the array in the first case? Hnnnn... I think it's a valid concern, because when processing streams, it makes it hard to handle special case of 1-element stream.Sniffing for
---still seems pretty bad (even if we made sure that it's a line on its own). First, I'm afraid of special cases. Second, IIUC formally it's not a magic symbol for streams, but an explicit start of the document.Perhaps an explicit
parseYamlStreamwhich always returns the array is the way out of this.@sparkprime @sh0rez Any thoughts about handling YAML streams?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's my concern. You are correct in that it attempts to read a single document, but should we treat a 1-element stream differently or not is the question? Should the output of the following 2 examples be different or the same?
What about this one?
I think I can be convinced either way to be honest.