Skip to content

Interpret precision as display width #4443

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

Merged
merged 10 commits into from
May 25, 2025
Merged

Conversation

nikhilreddydev
Copy link
Contributor

Fixes #4272

It now correctly considers precision as display width precision.
Please let me know, if I need to add any test cases as well.

Thanks!

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Please address inline comments and add a test case to format-test.cc

@nikhilreddydev
Copy link
Contributor Author

nikhilreddydev commented May 22, 2025

Thanks for the PR! Please address inline comments and add a test case to format-test.cc

Added the test case for introduced function dd25ea9.
Please review and happy to fix any further changes if any.
Sorry for the delay.

Comment on lines 214 to 216
EXPECT_EQ(
fmt::detail::count_code_points_with_display_width_precision("🐱🐱🐱", 5),
2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's test the output of fmt::format instead of this internal function to make sure that precision computation works end-to-end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done as part of 93fabd0

Thank you!

@vitaut vitaut merged commit 6d79757 into fmtlib:master May 25, 2025
45 checks passed
@vitaut
Copy link
Contributor

vitaut commented May 25, 2025

Merged, thanks!

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.

Use display width in precision
2 participants