yaml icon indicating copy to clipboard operation
yaml copied to clipboard

Upgrade ghodss/yaml to use go-yaml v3

Open silasdavis opened this issue 4 years ago • 13 comments

This is a breaking change because:

  • Strict mode is now the default for yaml.v3
  • String-valued boolean support has been dropped for YAML 1.2 spec compliance
  • Default indentation changes (we could set to previous value but since already breaking might make more sense to use base library default)

As such I have appended the /v2 suffix to the github.com/ghodss/yaml package name.

fixes #61

One major side-effect of this upgrade (and the one I am after) is that rountrip coding of YAML files does not incinerate comments.

Signed-off-by: Silas Davis [email protected]

silasdavis avatar Apr 22 '20 09:04 silasdavis

Thanks for looking into this @silasdavis!

tychedelia avatar Apr 23 '20 01:04 tychedelia

is there any release date planned for this?

kckommi avatar May 20 '20 03:05 kckommi

@bradfitz Sorry to ping you, but looks like you were the last person to merge a PR on this repo. Is it possible you could take a look at this? Not sure what the maintenance story is here. @ghodss seems to be MIA.

tychedelia avatar May 20 '20 19:05 tychedelia

I am also very interested in getting this merged, as it solves some serious issues in Helm.

david-l-riley avatar Aug 25 '20 14:08 david-l-riley

Just changing the module name doesn't seem right. We'd want a way to maintain both major branches.

bradfitz avatar Aug 31 '20 16:08 bradfitz

Like a top level /v1 and /v2 directory in the repo?

silasdavis avatar Sep 01 '20 12:09 silasdavis

I feel like it's traditional to have just the /v2 so that the original is properly backwards compatible. The other option would be having a long-term maintenance branch for v1 so that updates may be backported (this may ultimately be necessary anyway, though there should still be a /v2 directory for all the reasons described in the canonical blog post).

david-l-riley avatar Sep 08 '20 15:09 david-l-riley

Said blog post actually lays out a pretty logical practice for multi-version module maintenance which doesn't require a long-term maintenance branch.

david-l-riley avatar Sep 08 '20 15:09 david-l-riley

I'm hoping to find some time over the holidays to take the work @silasdavis did and migrate it into the appropriately versioned module.

tychedelia avatar Dec 11 '20 00:12 tychedelia

I would to suggest making v2 a pre-release version in order to make more important breaking changes without need to make v3. For example, it seems important to preserve the order of keys in a JSONToYAML transformation to fix #63. It's easy to implement do using the yaml v3 Node object. Look at my PoC:

import (
	"bytes"
	"fmt"
	"strings"

	"gopkg.in/yaml.v3"
)



// JSONToYAML transforms JSON to YAML keeping keys order.
func JSONToYAML(jsonData []byte) ([]byte, error) {
	return NewMarshaler().JSONToYAML(jsonData)
}
func NewMarshaler(os ...MarshalOption) *Marshaler {
	opts := &marshalOptions{
		Intend: 4, // Default for yaml.v3 package.
	}
	for _, o := range os {
		o(opts)
	}
	m := &Marshaler{}
	m.enc = yaml.NewEncoder(&m.buf)
	m.enc.SetIndent(opts.Intend)
	return m
}

type Marshaler struct {
	buf bytes.Buffer
	enc *yaml.Encoder
}

func (m *Marshaler) JSONToYAML(jsonData []byte) ([]byte, error) {
	n := &yaml.Node{}
	err := yaml.Unmarshal(jsonData, n)
	if err != nil {
		return nil, err
	}
	jsonToYAMLFormat(n)

	m.buf.Reset()
	err = m.enc.Encode(n)
	if err != nil {
		return nil, fmt.Errorf("marshal formated: %w", err)
	}
	return m.buf.Bytes(), nil
}

type MarshalOption func(opts *marshalOptions)
type marshalOptions struct {
	Intend int
}

func Indent(n int) MarshalOption {  return func(opts *marshalOptions) {opts.Intend = n } }

func jsonToYAMLFormat(n *yaml.Node) {
	if n == nil {
		return
	}
	switch n.Kind {
	case yaml.SequenceNode, yaml.MappingNode:
		n.Style = yaml.LiteralStyle
	case yaml.ScalarNode:
		if n.Style == yaml.DoubleQuotedStyle {
			n.Style = yaml.FlowStyle
			if strings.Contains(n.Value, "\n") {
				n.Style = yaml.LiteralStyle
			}
		}
	}
	for _, c := range n.Content {
		jsonToYAMLFormat(c)
	}
}

Seems idea of formatting *yaml.Node may be applied to get YAMLToJSON transformation too.

skipor avatar Dec 18 '20 15:12 skipor

Any movement on this. Would love to get rid of yaml.v2 from Podman, but need this to get merged.

rhatdan avatar Jun 17 '22 15:06 rhatdan

@bradfitz Please merge this so we can remove yaml.v2 from vendored directories.

rhatdan avatar Jan 27 '23 20:01 rhatdan