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

Fix for printing long doubles bug in dump_float #3929

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

RUSLoker
Copy link

@RUSLoker RUSLoker commented Jan 25, 2023

When you use long double as a floating point type with the current version of include/nlohmann/detail/output/serializer.hpp and try to dump json it prints trash instead of actual number. This if-else fixes the problem. On using long double you just need to add an 'L' modifier before 'g' in format string.

When you use long double as a floating point type with the current version of this file and try to dump json it prints trash instead of actual number. This if-else fixes the problem. On using long double you just need to add an 'L' modifier before 'g' in format string.
@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @RUSLoker Please read and follow the Contribution Guidelines.

@github-actions github-actions bot added the S label Jan 25, 2023
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

The library targets C++11. Please adjust the code.

@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @RUSLoker Please read and follow the Contribution Guidelines.

@github-actions
Copy link

🔴 Amalgamation check failed! 🔴

The source code has not been amalgamated. @RUSLoker Please read and follow the Contribution Guidelines.

@RUSLoker RUSLoker requested review from gregmarr and nlohmann and removed request for nlohmann and gregmarr January 26, 2023 08:46
@coveralls
Copy link

coveralls commented Jan 26, 2023

Coverage Status

coverage: 100.0%. remained the same
when pulling cf5c7fb on RUSLoker:patch-1
into a259ecc on nlohmann:develop.

Copy link
Contributor

@gregmarr gregmarr left a comment

Choose a reason for hiding this comment

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

LGTM, @nlohmann will need to merge.

@t-b
Copy link
Contributor

t-b commented Jan 26, 2023

I think a test is missing or?

@nlohmann
Copy link
Owner

Yes, a test is definitely required.

There is also #3906 that I need to take care of to finally have a green CI again. I'm busy right now, so sorry for delays.

@nlohmann
Copy link
Owner

Please update to recent develop version - the CI is fixed there (see #3906).

@nlohmann nlohmann added the please rebase Please rebase your branch to origin/develop label Jan 31, 2023
@RUSLoker
Copy link
Author

RUSLoker commented Feb 1, 2023

@nlohmann, it asks for your approval to run CI workflows.

@RUSLoker
Copy link
Author

RUSLoker commented Feb 1, 2023

@nlohmann, @gregmarr, what's wrong with Cirrus CI / check?

@nlohmann
Copy link
Owner

nlohmann commented Feb 1, 2023

@nlohmann, @gregmarr, what's wrong with Cirrus CI / check?

Looks like a glitch.

@nlohmann nlohmann removed the please rebase Please rebase your branch to origin/develop label Feb 1, 2023
Copy link
Owner

@nlohmann nlohmann left a comment

Choose a reason for hiding this comment

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

Please add tests.

@github-actions github-actions bot removed the S label Dec 15, 2023
@nlohmann
Copy link
Owner

Any update on this? Tests are still missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants