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

Preserve defaults while decoding nested structs #260

Merged
merged 2 commits into from
Nov 18, 2021
Merged

Preserve defaults while decoding nested structs #260

merged 2 commits into from
Nov 18, 2021

Conversation

Al2Klimov
Copy link
Contributor

@Al2Klimov Al2Klimov commented Oct 14, 2021

---
a: A
#b: DEFAULT # already works
c:
  d: D
  #e: DEFAULT # was not DEFAULT, but "", now works

@Al2Klimov
Copy link
Contributor Author

The new tests are failing, but due to the implementation:

$ go test ./...
--- FAIL: TestDecoder_DefaultValues (0.00s)
    decode_test.go:1781: v.J.K should be `defaultKValue`, got ``
FAIL
FAIL    github.com/goccy/go-yaml        0.150s
?       github.com/goccy/go-yaml/ast    [no test files]
?       github.com/goccy/go-yaml/cmd/ycat       [no test files]
?       github.com/goccy/go-yaml/internal/errors        [no test files]
ok      github.com/goccy/go-yaml/lexer  (cached)
ok      github.com/goccy/go-yaml/parser (cached)
ok      github.com/goccy/go-yaml/printer        (cached)
?       github.com/goccy/go-yaml/scanner        [no test files]
ok      github.com/goccy/go-yaml/token  (cached)
FAIL

With gopkg.in/yaml.v3 ...

$ git diff -U1
diff --git a/decode_test.go b/decode_test.go
index fb13b9a..07fc717 100644
--- a/decode_test.go
+++ b/decode_test.go
@@ -6,2 +6,3 @@ import (
        "fmt"
+       yaml2 "gopkg.in/yaml.v3"
        "io"
@@ -1752,3 +1753,3 @@ w: w_value
 `
-       if err := yaml.NewDecoder(strings.NewReader(src)).Decode(&v); err != nil {
+       if err := yaml2.NewDecoder(strings.NewReader(src)).Decode(&v); err != nil {
                t.Fatalf(`parsing should succeed: %s`, err)
diff --git a/go.mod b/go.mod
index 4d99408..2248eeb 100644
--- a/go.mod
+++ b/go.mod
@@ -9,2 +9,3 @@ require (
        golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
+       gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
 )
diff --git a/go.sum b/go.sum
index 8ce2a41..8d090ed 100644
--- a/go.sum
+++ b/go.sum
@@ -41 +41,3 @@ gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo=
+gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

... they don't fail.

@Al2Klimov Al2Klimov changed the title TestDecoder_DefaultValues(): also cover nested structs Preserve defaults while decoding nested structs Oct 15, 2021
@Al2Klimov
Copy link
Contributor Author

@goccy Now it should work!

@codecov-commenter
Copy link

Codecov Report

Merging #260 (c6bcb80) into master (b478465) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
+ Coverage   76.44%   76.48%   +0.03%     
==========================================
  Files          12       12              
  Lines        4216     4222       +6     
==========================================
+ Hits         3223     3229       +6     
  Misses        759      759              
  Partials      234      234              

Copy link

@htriem htriem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@goccy
Copy link
Owner

goccy commented Nov 17, 2021

@Al2Klimov
Could you please describe this fixing in this PR description with example YAML.

@Al2Klimov
Copy link
Contributor Author

@goccy Done.

@goccy
Copy link
Owner

goccy commented Nov 18, 2021

@Al2Klimov Thank you ! LGTM

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.

4 participants