Skip to content

INI: fix converting integers to strings; improve float and time.Time formatting #1929

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
Merged
merged 4 commits into from
Sep 9, 2025
Merged

INI: fix converting integers to strings; improve float and time.Time formatting #1929

merged 4 commits into from
Sep 9, 2025

Conversation

felixfontein
Copy link
Contributor

@felixfontein felixfontein commented Aug 30, 2025 •
edited
Loading

Before:

[section]
int       = %!s(int=123)
float     = 1.230000
bool      = true
date      = 2025-01-01 00:00:00 +0000 UTC
timestamp = 2025-01-01 01:02:03 +0000 UTC
string    = this is a string

After first commit:

[section]
int       = 123
float     = 1.230000
bool      = true
date      = 2025-01-01T00:00:00Z
timestamp = 2025-01-01T01:02:03Z
string    = this is a string

Latest version:

[section]
int       = 123
float     = 1.23
bool      = true
date      = 2025-01-01T00:00:00Z
timestamp = 2025-01-01T01:02:03Z
string    = this is a string

Fixes #1915.

@felixfontein felixfontein requested a review from a team August 30, 2025 09:54
@felixfontein felixfontein mentioned this pull request Aug 30, 2025
Closed
@felixfontein
Copy link
Contributor Author

@getsops/maintainers this change is a breaking change, since it changes the output. For integers, I think this is definitely OK. Timestamps haven't been supported for that long, and I think the new output is a lot better, and I think there it's definitely also OK. For floats, I think the old output was bad since it potentially destroyed the value (like 1.2345E-10 getting turned into 0). What do you think, is this OK for a new feature release?

@felixfontein felixfontein changed the title Fix converting integers to strings; also improve time.Time formatting Fix converting integers to strings; improve float and time.Time formatting Aug 30, 2025
@felixfontein felixfontein changed the title Fix converting integers to strings; improve float and time.Time formatting INI: fix converting integers to strings; improve float and time.Time formatting Aug 30, 2025
@billy4479 billy4479 mentioned this pull request Sep 2, 2025
Merged
sabre1041
sabre1041 approved these changes Sep 9, 2025
View reviewed changes
Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@felixfontein
Fix converting integers to strings. Also improve time.Time formatting.
d67d29f 
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein
Add tests.
2e2d7d9 
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein
Improve formatting of floats.
816b797 
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein
Move ValToString to stores.
277805a 
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein felixfontein merged commit 193c269 into getsops:main Sep 9, 2025
16 checks passed
@felixfontein
Copy link
Contributor Author

@sabre1041 thanks a lot for reviewing this!

@felixfontein felixfontein deleted the ini-bug branch September 9, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sabre1041 sabre1041 sabre1041 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

INI store: conversion to string destroys integers
2 participants
@felixfontein @sabre1041