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.

Sign up for GitHub

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

Jump to bottom

bug: toYaml should omit empty fields similar to toJson #80

Open
1 task done
mrueg opened this issue Nov 27, 2024 · 5 comments · May be fixed by #96
Open
1 task done

bug: toYaml should omit empty fields similar to toJson #80

mrueg opened this issue Nov 27, 2024 · 5 comments · May be fixed by #96
Assignees
@42atomys
Labels
aspect/dex 🤖 Concerns developers' experience with the codebase state/needs information 🚧 We needs more information to go forwards

Comments

@mrueg
Copy link
Contributor

mrueg commented Nov 27, 2024

You have a proposal, explain it!

Right now toYaml represents the whole object. It would be great if there were an option to allow it to ignore empty fields.

Describe the solution you'd like

Have new toYamlWithParams function maybe with #79 that allows setting omitting empty fields as well as setting indentation.

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@mrueg mrueg added the state/triage 🚦 Has not been triaged & therefore, not ready for work label Nov 27, 2024
@mrueg
Copy link
Contributor Author

mrueg commented Nov 27, 2024

What makes the empty fields go away is the following pipeline: {{ .Object |toJson | fromJson | toYaml }} as json seems to omit the empty fields by default.

@42atomys
Copy link
Member

Hello, good catch, toYaml needs to follow the same logic as toJson, I will tag this one as bug and works on it soon :)

Thanks for your report

@42atomys 42atomys added good first issue Good for newcomers state/todo 🚀 This is confirmed, will work on soon domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously aspect/dex 🤖 Concerns developers' experience with the codebase priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon labels Nov 27, 2024
@mrueg mrueg changed the title proposal: toYaml should allow to omit empty fields bug: toYaml should omit empty fields similar to toJson Nov 27, 2024
@42atomys 42atomys removed the state/triage 🚦 Has not been triaged & therefore, not ready for work label Nov 27, 2024
@42atomys 42atomys added this to the v1.0-rc.3 milestone Nov 27, 2024
@42atomys 42atomys linked a pull request Dec 2, 2024 that will close this issue
test: add test case for struct input with struct tags #96
Draft
6 tasks
@42atomys 42atomys linked a pull request Dec 2, 2024 that will close this issue
Draft
6 tasks
@42atomys
Copy link
Member

42atomys commented Dec 2, 2024

@mrueg Hi thanks for your patience, I have add a test on #96, but the omitempty and - seams to be resolve normally, can you have an example to provide ?

@mrueg
Copy link
Contributor Author

mrueg commented Dec 2, 2024

Thanks for creating a test cases. Looking again at it, I can't reproduce it in a minimal example (tried a few and it works as expected).
I'm using it in a library that does a lot of things with the struct in between (e.g. json marshalling and unmarshalling), so I'll need to take a look deeper and hopefully can reproduce it then.

@42atomys 42atomys added state/needs information 🚧 We needs more information to go forwards priority/low 🟩 Priority 4 - Low priority and doesn't need to be rushed domain/complicated 🟨 The relationship between cause and effect requires analysis or expertise and removed state/todo 🚀 This is confirmed, will work on soon domain/obvious 🟩 Represents the "known knowns" issue. It's Obviously priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon labels Dec 3, 2024
@42atomys
Copy link
Member

42atomys commented Dec 3, 2024

Ok, thanks for your answer, I put it as state/needs information 🚧 We needs more information to go forwards to help triage. Come back if you found a reproduction procedure !

@42atomys 42atomys removed good first issue Good for newcomers priority/low 🟩 Priority 4 - Low priority and doesn't need to be rushed domain/complicated 🟨 The relationship between cause and effect requires analysis or expertise labels Dec 6, 2024
@42atomys 42atomys modified the milestones: v1.0-rc.3, v1.0 Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@42atomys 42atomys

Labels
aspect/dex 🤖 Concerns developers' experience with the codebase state/needs information 🚧 We needs more information to go forwards
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: add test case for struct input with struct tags go-sprout/sprout
2 participants
@mrueg @42atomys