-
Notifications
You must be signed in to change notification settings - Fork 39
fix(overlay): keep folded block scalars stable across repeated applies #2116
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package overlay | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| // Block indentation is stripped on decode, so leading whitespace means more-indented. | ||
| func hasMoreIndentedLine(value string) bool { | ||
| for _, line := range strings.Split(value, "\n") { | ||
| if line == "" { | ||
| continue | ||
| } | ||
| if line[0] == ' ' || line[0] == '\t' { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| // yaml.v3 injects a line break before more-indented lines in folded scalars on every encode; literal blocks round trip unchanged. | ||
| func stabilizeFoldedScalars(node *yaml.Node) { | ||
| if node == nil { | ||
| return | ||
| } | ||
|
|
||
| if node.Kind == yaml.ScalarNode && node.Style == yaml.FoldedStyle && hasMoreIndentedLine(node.Value) { | ||
| node.Style = yaml.LiteralStyle | ||
| } | ||
|
|
||
| for _, child := range node.Content { | ||
| stabilizeFoldedScalars(child) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| package overlay | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| "gopkg.in/yaml.v3" | ||
| ) | ||
|
|
||
| const foldedWithMoreIndentedLine = `description: >- | ||
| ### Authorization | ||
|
|
||
| | Type | Scopes | | ||
| | ---- | ------ | | ||
| | Organization | ` + "`manage_passwords`" + ` | | ||
| ` | ||
|
|
||
| func roundTrip(t *testing.T, doc string, stabilize bool) string { | ||
| t.Helper() | ||
|
|
||
| var node yaml.Node | ||
| require.NoError(t, yaml.Unmarshal([]byte(doc), &node)) | ||
|
|
||
| if stabilize { | ||
| stabilizeFoldedScalars(&node) | ||
| } | ||
|
|
||
| out, err := yaml.Marshal(&node) | ||
| require.NoError(t, err) | ||
|
|
||
| return string(out) | ||
| } | ||
|
|
||
| func decodeDescription(t *testing.T, doc string) string { | ||
| t.Helper() | ||
|
|
||
| var decoded struct { | ||
| Description string `yaml:"description"` | ||
| } | ||
| require.NoError(t, yaml.Unmarshal([]byte(doc), &decoded)) | ||
|
|
||
| return decoded.Description | ||
| } | ||
|
|
||
| func TestStabilizeFoldedScalarsSurvivesRepeatedApplies(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| want := decodeDescription(t, foldedWithMoreIndentedLine) | ||
|
|
||
| doc := foldedWithMoreIndentedLine | ||
| for i := range 30 { | ||
| doc = roundTrip(t, doc, true) | ||
| assert.Equal(t, want, decodeDescription(t, doc), "value changed after %d applies", i+1) | ||
| } | ||
| } | ||
|
|
||
| // Fails once gopkg.in/yaml.v3 fixes the emitter, at which point stabilizeFoldedScalars can go. | ||
| func TestFoldedScalarGrowsWithoutStabilizer(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| before := decodeDescription(t, foldedWithMoreIndentedLine) | ||
| after := decodeDescription(t, roundTrip(t, foldedWithMoreIndentedLine, false)) | ||
|
|
||
| assert.NotEqual(t, before, after) | ||
| } | ||
|
|
||
| func TestStabilizeFoldedScalarsLeavesStableStylesAlone(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| doc string | ||
| }{ | ||
| { | ||
| name: "folded scalar with no more-indented line", | ||
| doc: "description: >-\n one line\n another line\n", | ||
| }, | ||
| { | ||
| name: "literal scalar with more-indented line", | ||
| doc: "description: |-\n one line\n more indented\n", | ||
| }, | ||
| { | ||
| name: "plain scalar", | ||
| doc: "description: just a string\n", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| want := decodeDescription(t, tt.doc) | ||
|
|
||
| doc := tt.doc | ||
| for range 5 { | ||
| doc = roundTrip(t, doc, true) | ||
| assert.Equal(t, want, decodeDescription(t, doc)) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestHasMoreIndentedLine(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| value string | ||
| want bool | ||
| }{ | ||
| {name: "no indentation", value: "one\ntwo", want: false}, | ||
| {name: "space indented line", value: "one\n two", want: true}, | ||
| {name: "tab indented line", value: "one\n\ttwo", want: true}, | ||
| {name: "blank lines only", value: "one\n\ntwo", want: false}, | ||
| {name: "empty", value: "", want: false}, | ||
| {name: "first line indented", value: " one\ntwo", want: true}, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| assert.Equal(t, tt.want, hasMoreIndentedLine(tt.value)) | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -120,6 +120,8 @@ func apply(document *yaml.Node, o *overlay.Overlay, sourceLocation string, yamlI | |
| } | ||
| } | ||
|
|
||
| stabilizeFoldedScalars(document) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When a folded scalar starts with a blank line and also contains a more-indented line, this call changes its decoded value on the first apply. Preserve folded style for leading-newline values or update the stabilizer to emit a literal block that retains the initial blank line. Prompt for AI agents |
||
|
|
||
| bytes, err := schemas.RenderDocument(document, sourceLocation, yamlIn, yamlOut) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to Render document: %w", err) | ||
|
|
||
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.
P2: Explicitly tagged folded scalars bypass stabilization because
Styleis a bitmask and includesTaggedStyle. Test the folded bit and replace only that bit while preserving the tag, so tagged descriptions cannot keep gaining blank lines.Prompt for AI agents