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

from_chars_advanced overload function taking parsed_number_string_t #249

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zejal
Copy link

@zejal zejal commented May 13, 2024

Idea is to split parsing from value computation adding a from_chars_advanced function taking result of parsing.

@lemire
Copy link
Member

lemire commented May 13, 2024

@zejal Can you motivate this change? Why is it beneficial?

@zejal
Copy link
Author

zejal commented May 13, 2024

This would allow someone (as I would like to do to use fast_float in existing code) to write an alternative string parser function that creates and populates the parsed_number_string_t and call added overloaded function. Of course the existing from_chars_advanced taking chars range and parsing options remains and called suggested added one.

Perhaps accepting this change implies to document a bit what the contents of parsed_number_string_t structure. In particular the integer and fraction bits ?

@lemire
Copy link
Member

lemire commented May 13, 2024

Perhaps accepting this change implies to document a bit what the contents of parsed_number_string_t structure. In particular the integer and fraction bits ?

Do you want to add some comments to the code so that this is clearer? No need to write very much, just enough so that people understand why the code is structured this way.

@zejal
Copy link
Author

zejal commented May 13, 2024

Thanks, added some comments, hope it helps.

@lemire
Copy link
Member

lemire commented May 13, 2024

I expect to merge once the test pass.

It should be part of the next release.

@zejal
Copy link
Author

zejal commented May 13, 2024

Sure, thanks a lot

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

Successfully merging this pull request may close these issues.

None yet

2 participants