-
Notifications
You must be signed in to change notification settings - Fork 624
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
limayaml.Save does have some encoding bugs #2665
Comments
Thx for mention ;) |
The main problem was the mount of the home directory, So it was the same issue as "null", even if I don't think the strings "null" (or "Null" or "NULL") are used: |
jandubois
added a commit
to jandubois/lima
that referenced
this issue
Nov 1, 2024
We used a custom marshaller to properly marshal "~" and "null". This has been fixed in goccy/go-yaml#474, which has been merged in lima-vm#2828. Dropping the custom marshaller fixes lima-vm#2665. Signed-off-by: Jan Dubois <[email protected]>
subpop
pushed a commit
to subpop/lima
that referenced
this issue
Dec 4, 2024
We used a custom marshaller to properly marshal "~" and "null". This has been fixed in goccy/go-yaml#474, which has been merged in lima-vm#2828. Dropping the custom marshaller fixes lima-vm#2665. Signed-off-by: Jan Dubois <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The function uses a custom marshaller to make sure
null
and~
are quoted. It is missing entries forNull
andNULL
which in the YAML spec also denote the missing value.I found that tab characters get an extra backslash during the roundtrip (I've filed Custom marshalling does incorrect escaping of some control characters goccy/go-yaml#475) for this. This bug only happens when using a custom marshaller, but not with the default one.
So (1) can be fixed by added the missing cases, but (2) can only be fixed by removing the custom marshaller, making it impossible to fix (1).
But by lucky synchronicity a PR has been filed just 10 hours ago to fix the
null
issue upstream (goccy/go-yaml#474). I've tested that branch, and it fixes both (1) and (2) when you remove the custom marshaller.So it is possible to fix the issue right away with a
replace
directive ingo.mod
and dropping the custom marshaller:I don't think this is important enough, and we should instead wait a while to see if upstream makes a release. We could fix (1) while we wait, if we want to.
The text was updated successfully, but these errors were encountered: