Fix to work with perls since 5.37.7 #13
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This fixes Perl/perl5#20571 The commit blamed in that tickect caused POSIX::localeconv() to return all the defined fields in the lconv structure. Number::Format had been relying on it to suppress empty fields. The solution is simply for Number::Format to instead ignore empty fields itself. This is entirely backwards compatible, as previously those fields were never delivered to Number::Format. There is a slight complication in that the POSIX standard allows for the monetary decimal point and the monetary thousands separator to both be empty. There is a check in Number::Format against both being empty that must be relaxed to allow this legal combination. The check previously was effectively useless as Number::Format would never see those fields if they were empty, as POSIX::localeconv() would suppress them.
The comments wrongly claimed that certain platform behaviors were broken, whereas they are legal, and the module just needed to handle them. This commit updates the text to better describe the situation.
khwilliamson
referenced
this pull request
in Perl/perl5
Jan 31, 2023
The blamed commit, 04de022, exposed a bug in the module itself. I will submit a PR to fix it. But this ticket did tell me that there was a problem with that commit. It returned a C language value, CHAR_MAX, which doesn't really have a corresponding concept in Perl. Instead we use -1 to indicate that a positive-valued variable is in some abnormal state. This commit changes to do that, and documents the changes, which should have been done in 04de022.
|
(Bump.) I have sent Bill a direct email as well, offering to make a release. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes Perl/perl5#20571
The commit blamed in that tickect caused POSIX::localeconv() to return
all the defined fields in the lconv structure. Number::Format had been
relying on it to suppress empty fields.
The solution is simply for Number::Format to instead ignore empty fields
itself. This is entirely backwards compatible, as previously those
fields were never delivered to Number::Format.
There is a slight complication in that the POSIX standard allows for
the monetary decimal point and the monetary thousands separator to both
be empty. There is a check in Number::Format against both being empty
that must be relaxed to allow this legal combination. The check
previously was effectively useless as Number::Format would never see
those fields if they were empty, as POSIX::localeconv() would suppress
them.