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

Upgrade ghodss/yaml to use go-yaml v3 #62

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

Conversation

silasdavis
Copy link

@silasdavis silasdavis commented Apr 22, 2020

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 silasdavis changed the title This upgrade ghodss/yaml to use go-yaml v3 Upgrade ghodss/yaml to use go-yaml v3 Apr 22, 2020
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.

Signed-off-by: Silas Davis <[email protected]>
@tychedelia
Copy link

tychedelia commented Apr 23, 2020

Thanks for looking into this @silasdavis!

@kckommi
Copy link

kckommi commented May 20, 2020

is there any release date planned for this?

@tychedelia
Copy link

@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.

@david-l-riley
Copy link

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

@bradfitz
Copy link
Collaborator

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

@silasdavis
Copy link
Author

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

@david-l-riley
Copy link

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
Copy link

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

@tychedelia
Copy link

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

@skipor
Copy link

skipor commented Dec 18, 2020

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.

@rhatdan
Copy link

rhatdan commented Jun 17, 2022

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

@rhatdan
Copy link

rhatdan commented Jan 27, 2023

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

justinsb pushed a commit to justinsb/yaml that referenced this pull request Jan 31, 2023
Bump gopkg.in/yaml.v2 to v2.4.0: fixes inconsistent long lines wrapping
@diamondburned
Copy link

@skipor Thank you for this proof-of-concept! I opened a PR to this PR that does exactly this: silasdavis#1.

diamondburned pushed a commit to diamondburned/kin-openapi that referenced this pull request Aug 12, 2024
This commit adds the `OrderedPropertyKeys` method to the
`openapi3.Schema`:

    OrderedPropertyKeys returns the keys of the properties in the order
    they were defined. This is useful for generating code that needs to
    iterate over the properties in a consistent order. If the keys could
    not be extracted for some reason, then this method automatically
    sorts the keys to be deterministic.

This is done via a temporary fork of the YAML-to-JSON transformation library.
It will not be ready until these PRs are merged in this order:

    - silasdavis/yaml#1
    - ghodss/yaml#62
diamondburned added a commit to diamondburned/kin-openapi that referenced this pull request Aug 12, 2024
This commit adds the `OrderedPropertyKeys` method to the
`openapi3.Schema`:

    OrderedPropertyKeys returns the keys of the properties in the order
    they were defined. This is useful for generating code that needs to
    iterate over the properties in a consistent order. If the keys could
    not be extracted for some reason, then this method automatically
    sorts the keys to be deterministic.

This is done via a temporary fork of the YAML-to-JSON transformation library.
It will not be ready until these PRs are merged in this order:

    - silasdavis/yaml#1
    - ghodss/yaml#62
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yaml.v3 support
8 participants