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

Address goccy/go-yaml#345 #346

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Address goccy/go-yaml#345 #346

wants to merge 1 commit into from

Conversation

coxley
Copy link

@coxley coxley commented Feb 20, 2023

OrderedMap values aren't recording their comments like normal maps. Reproduced using the example in the linked issue, copied below:

package main

import (
	"bytes"
	"fmt"
	"io"

	"github.com/goccy/go-yaml"
)

var contents = []byte(`
top:
  # This comment is captured
  first: true
  # But not this one
  second:
    # But this one is
    first: true
    # But not this one
    second: true
    # Neither this
    third: true
`)

func main() {
	cm := yaml.CommentMap{}
	dec := yaml.NewDecoder(bytes.NewReader(contents), yaml.UseOrderedMap(), yaml.CommentToMap(cm))

	for {
		var mslice yaml.MapSlice
		err := dec.Decode(&mslice)
		if err == io.EOF {
			break
		}

		if err != nil {
			panic(err)
		}
	}

	for _, comment := range cm {
		fmt.Println(comment.Texts)
	}
}

Before this change, the bug exists. With this change, it's fixed.

OrderedMap values aren't recording their comments like normal maps.
@goccy
Copy link
Owner

goccy commented Mar 8, 2023

@coxley Thank you for your reporting. The changes looks good, please add the test case.

@goccy goccy added the reviewed label Mar 19, 2023
@goccy
Copy link
Owner

goccy commented Mar 28, 2023

@coxley Do you have time to fix this ? If you can't take the time, I will take care of it.

@coxley
Copy link
Author

coxley commented Jun 20, 2023

@goccy Sorry! I got pulled away into other things, and stopped using go-yaml.

One thing I dislike about my company using Github is that work notifications bury open source ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants