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

<chrono>: Fix formatter ignoring dynamically provided width #4283

Merged
merged 4 commits into from
Jan 11, 2024

Conversation

mt3d
Copy link
Contributor

@mt3d mt3d commented Dec 22, 2023

Fixes #4201.

As has @cpplearner noted, _Chrono_formatter::_Write() calls and returns _Write_aligned before trying to check if a width is provided dynamically. The updated behavior is now similar to that of _Formatter_base::format().

@mt3d mt3d requested a review from a team as a code owner December 22, 2023 10:43
@StephanTLavavej StephanTLavavej self-assigned this Jan 10, 2024
@StephanTLavavej StephanTLavavej added bug Something isn't working format C++20/23 format chrono C++20 chrono labels Jan 10, 2024
stl/inc/chrono Outdated Show resolved Hide resolved
tests/std/tests/GH_004201_chrono_formatter/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jan 11, 2024
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug and adding a test! 😻 I pushed a commit to fix minor nitpicks.

We simultaneously merge PRs to GitHub and our MSVC-internal repo; this is a semi-manual process so we batch up PRs to save time. Your PR will be part of the next batch, likely in the next couple of days.

@StephanTLavavej StephanTLavavej self-assigned this Jan 11, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej
Copy link
Member

I've pushed a merge with main to resolve a trivial adjacent-add conflict in tests/std/test.lst with #4275.

@StephanTLavavej StephanTLavavej merged commit 629216a into microsoft:main Jan 11, 2024
35 checks passed
@StephanTLavavej
Copy link
Member

Thanks for fixing this bug in an important C++20 feature, and congratulations on your first microsoft/STL commit! 🛠️ 🎉 😻

This is expected to ship in VS 2022 17.10 Preview 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chrono C++20 chrono format C++20/23 format
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<chrono> <format>: duration formatting ignores dynamic width
2 participants