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

replacing values with a multi-line string value provides broken yaml format. #447

Open
mandelsoft opened this issue Apr 23, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@mandelsoft
Copy link

mandelsoft commented Apr 23, 2024

Describe the bug

The multi line string is replaced, but the indentation of the string lines is wrong.

To Reproduce

use yaml document:

data:
  value1: orig1
  value2: orig2

replace $.data.value1 by

line1
line2

The result will be

data:
  value1: |-
  line1
  line2
  value2: orig2

instead of

data:
  value1: |-
    line1
    line2
  value2: orig2

The data is parsed with parser.ParseBytes(data, 0).
The replace operation is

file, err := parser.ParseBytes(value, 0)
if err != nil {
	return err
}
p, err := yaml.PathString("$.data.value1")
if err != nil {
	return err
}
return p.ReplaceWithFile(doc, file)

Expected behavior
provide correct multi-line result

Screenshots
If applicable, add screenshots to help explain your problem.

Version Variables

  • Go version: 1.22
  • go-yaml's Version: v1.11.3

Additional context

The ast nodes provide some strange column values for string value. Especially there is no column info for
follow-up value lines. Therefore, the String() method just uses the original string formatting from the
target value, which will for sure be wrong in most cases. When replacing a value, only the column value is adapted for the new value, but this cannot be used to serialize follow-up lines.

@Skarlso
Copy link

Skarlso commented Apr 24, 2024

#292

Also reported two years ago without any resolution. :(

@Skarlso
Copy link

Skarlso commented Apr 24, 2024

I'm attempting to see if I can fix this.

@Skarlso
Copy link

Skarlso commented Apr 24, 2024

Reproduced the issue in path_test.go with:

		{
			path: "$.data.value1",
			dst: `
data:
  value1: orig1
  value2: orig2
`,
			src: `
line1
line2
`,
			expected: `
data:
  value1: |-
    line1
    line2
  value2: orig2
`,
		},

I'll attempt to fix the problem.

@Skarlso
Copy link

Skarlso commented Apr 24, 2024

The AST node replace doesn't care how deep it is so it plain does a replace on the node value resulting in a YAML that doesn't work.

@Skarlso
Copy link

Skarlso commented Apr 24, 2024

Tomorrow I'll try to fix this. I have an idea of tracking the deepness recursively for StringNodes for multi-line text. let's see..

@Skarlso
Copy link

Skarlso commented Apr 25, 2024

so I figured out that the library just expects you to pass in the value you require including the right indentation level.

		{
			path: "$.data.value1.orig1",
			dst: `
data:
  value1:
    orig1: orig1.1
  value2: orig2
`,
			src: `|-
      line1
      line2
`,
			expected: `
data:
  value1:
    orig1: |-
      line1
      line2
  value2: orig2
`,
		},

Passes as a test case.

@dee0
Copy link

dee0 commented Apr 25, 2024

Nice find @Skarlso
Can something similar be done when the new value of orig1 is an object instead of a multiline string? I ask because when I stumbled on this problem I was seeing the same problem for an object.

@Skarlso
Copy link

Skarlso commented Apr 25, 2024

Do you have an example?

@Skarlso
Copy link

Skarlso commented Apr 26, 2024

I tried replacing objects, but the marshalling is all over the place. There is virtually no consistency in indentation whatsoever. I can't find a pattern or anything that would make it work. :(

@Skarlso
Copy link

Skarlso commented Apr 26, 2024

Okay, this now worked finally:

		{
			path: "$.data.value1.orig1",
			dst: `
data:
  value1:
    orig1: orig1.1
  value2: orig2
`,
			src: `
orig1:
  newValue:
    - value1
    - value2
`,
			expected: `
data:
  value1:
    orig1:
      orig1:
        newValue:
          - value1
          - value2
  value2: orig2
`,
		},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants