-
Notifications
You must be signed in to change notification settings - Fork 1.1k
compact seq indent option #753
Conversation
go.sum
Outdated
| @@ -0,0 +1,4 @@ | |||
| gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= | |||
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 noticed that @niemeyer didn't have a go.sum. It might be that the build or whatever else produces it. I would not introduce this file, it seems like an unrelated change.
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.
done
yaml.go
Outdated
|
|
||
| // DisableSeqIndent disables the indentation for sequence items. | ||
| func (e *Encoder) DisableSeqIndent() { | ||
| e.encoder.emitter.disable_sequence_indent = true |
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.
is there a way this can be called prior to the encoder and emitter being instantiated?
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.
good question, though I'm wondering why we would want to do that?
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 don't think we wold want to do that, but in general as a package designer one should anticipate that people will do things that don't make sense. Is there a way to run into trouble with this interface? Can we avoid it? My PR had a flag on the current struct and then set one on the internal struct at init().
I guess it's not a huge deal and we can see if people break themselves first.
e965076 to
77123da
Compare
d648d2e to
9866fd3
Compare
|
Still interested in seeing this merged, but closing due to lack of activity. Please let me know if you have any interest in continuing this discussion. |
fixes #720, needed for kustomize to avoid breaking changes when upgrading our yaml v3 dependency.
This PR also includes a needed fix for #755 in its last commit, but I can remove it if preferred. The commit in #756 is the same as the last commit in this PR.
Happy to update the PR to expose the flag in a different way if that's desired.
Same questions as #750: