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

Bug: format decimal with thousands sep #353

Closed
shestakovda opened this issue Jul 11, 2016 · 1 comment
Closed

Bug: format decimal with thousands sep #353

shestakovda opened this issue Jul 11, 2016 · 1 comment

Comments

@shestakovda
Copy link

Hi! Thanks for good lib!

But I think there are some bugs, is that so?

Test code:

    setlocale(LC_NUMERIC, "en_US.UTF-8");
    // good
    assert(fmt::format("{:n}", 123) == "123");
    assert(fmt::format("{:n}", 12345) == "12,345");
    assert(fmt::format("{:n}", 123456) == "123,456");
    assert(fmt::format("{:n}", 1234567) == "1,234,567");
    assert(fmt::format("{:n}", 12345678) == "12,345,678");
    assert(fmt::format("{:n}", 123456789) == "123,456,789");
    // fail
    std::cout << fmt::format("{:n}", 1234) << std::endl;  // 11234
    assert(fmt::format("{:n}", 1234) == "1,234");
    std::cout << fmt::format("{:n}", 1234567890) << std::endl; // 11234,567,890
    assert(fmt::format("{:n}", 1234567890) == "1,234,567,890");

Seems to be an error in format.h:936: no thousands sep functor call. I think that it makes buffer to miss pointer position. This fix it for me (in format.h:934):

  unsigned index = static_cast<unsigned>(value * 2);
  *--buffer = Data::DIGITS[index + 1];
  thousands_sep(buffer);
  *--buffer = Data::DIGITS[index];
  thousands_sep(buffer);

And separator is "\xc2\xa0"; for "ru_RU.UTF-8" locale, so there is another mistake in format.h:2784 (need some additional brackets). The next code fixes it:

    unsigned size = static_cast<unsigned>(
          num_digits + sep.size() * ((num_digits - 1) / 3) );
@vitaut
Copy link
Contributor

vitaut commented Jul 11, 2016

Should be fixed in 0e6df7e. Thanks a lot for catching these!

@vitaut vitaut closed this as completed Jul 11, 2016
foonathan pushed a commit that referenced this issue Oct 23, 2016
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

No branches or pull requests

2 participants