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

Encode: make floats always contain a decimal point #615

Merged
merged 2 commits into from
Oct 14, 2021

Conversation

zostay
Copy link
Contributor

@zostay zostay commented Oct 4, 2021

Issue: #571

This is a fix to the integer to float conversion mentioned in #571.

This is breaking a test that seems to be ensuring that the existing behavior is preserved, so it's not ready for a merge. I didn't really know what to make of what this was testing, so if you'll let me know what this test is supposed to accomplish or how to correct it I will.

--- FAIL: TestUnmarshalCheckConversionFloatInt (0.00s)
    --- FAIL: TestUnmarshalCheckConversionFloatInt/float (0.00s)
        unmarshal_imported_test.go:1078: 
                Error Trace:    unmarshal_imported_test.go:1078
                Error:          An error is expected but got nil.
                Test:           TestUnmarshalCheckConversionFloatInt/float
FAIL
FAIL    github.com/pelletier/go-toml/v2/internal/imported_tests 0.308s

Also, I looked into adding support for unmarshalling into complex64 and complex128, but since you can't marshal those, that seemed a bit pointless. As far as I can tell, this is the only integer conversion missing. I suspect the reverse conversion is not a problem, but I did not look into it.

@zostay zostay changed the base branch from master to v2 October 4, 2021 17:40
Copy link
Owner

@pelletier pelletier left a comment

Choose a reason for hiding this comment

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

Thanks for your interest in go-toml! I think the problem here is not to accept int to float as that could cause trouble (both v1 and BurntSushi's don't do that), but rather when using a float that can be represented as an integer, the marshaler emits what looks like an integer, and therefore the decoder the interprets it as an integer. The goal should be that a given struct can take a round trip (marshal then unmarshal that output).

@zostay
Copy link
Contributor Author

zostay commented Oct 7, 2021

Ah, I misunderstood the problem. Easy fix, though.

@pelletier pelletier merged commit 4984dcb into pelletier:v2 Oct 14, 2021
@pelletier
Copy link
Owner

Sorry for the late reply! Merged, thank you for the patch!

@pelletier pelletier added the bug Issues describing a bug in go-toml. label Oct 28, 2021
@pelletier pelletier changed the title Fix error with Integer to Float conversion in v2 Encode: make float always contain a decimal point Oct 28, 2021
@pelletier pelletier changed the title Encode: make float always contain a decimal point Encode: make floats always contain a decimal point Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing a bug in go-toml.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants