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

Add visualizer for std::chrono::system_clock::time_point #5005

Merged

Conversation

Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Oct 8, 2024

Adds a visualizer for std::chrono::system_clock::time_point. To compute the date representation, it uses the same algorithm as _Civil_from_days (with the same "variables" as https://howardhinnant.github.io/date_algorithms.html#civil_from_days).
The time is displayed as hh:mm:ss.ssssss. Additionally, the visualizer shows ns/100 (the actual value of the time-point), us, ms, and s.

Unfortunately, the format specifiers don't provide a way of formatting numbers with padding, so they're formatted "manually" (e.g. a 2-digit v is formatted as {v/10}{v%10}).

See also: #314

@Nerixyz Nerixyz requested a review from a team as a code owner October 8, 2024 17:39
@StephanTLavavej StephanTLavavej added the visualizer How the VS debugger displays STL types label Oct 8, 2024
@StephanTLavavej
Copy link
Member

  • Can you show a screenshot?
  • Because you're manually formatting numbers, is this totally wrecked by Hexadecimal Display? Can that be avoided with a decimal format specifier?

@Nerixyz
Copy link
Contributor Author

Nerixyz commented Oct 8, 2024

  • Can you show a screenshot?
VS
VS (hex)
VS Code
  • Because you're manually formatting numbers, is this totally wrecked by Hexadecimal Display? Can that be avoided with a decimal format specifier?

Yes - I updated it to use decimal formatting.

@Nerixyz Nerixyz force-pushed the feat/chrono-timepoint-natvis branch from fb061d7 to c4bac42 Compare October 8, 2024 18:38
<Intrinsic Name="era" Expression="(z() &gt;= 0 ? z() : z() - 146096) / 146097"/>
<Intrinsic Name="doe" Expression="(unsigned)(z() - era() * 146097)"/>
<Intrinsic Name="yoe" Expression="(doe() - doe()/1460 + doe()/36524 - doe()/146096) / 365"/>
<Intrinsic Name="doy" Expression="doe() - (365*yoe() + yoe()/4 - yoe()/100)"/>
Copy link
Member

Choose a reason for hiding this comment

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

No change requested: Stylistically, I would have put spaces around all binary operators, as we do in product code (enforced by clang-format). But I can see an argument for the choices here, and I'm not picky enough about visualizer syntax for it to be worth resetting testing.

@StephanTLavavej
Copy link
Member

Thanks, this is awesome! 😻

It didn't matter for this PR, but for the future - we recommend against force-pushing after review has begun, as GitHub makes it more difficult to see what's incrementally changed. (Force-pushing is okay if there's a solid reason for it, like reorganizing commits to make them comprehensible, or if review hasn't started yet.)

@CaseyCarter CaseyCarter removed their assignment Oct 21, 2024
@StephanTLavavej StephanTLavavej self-assigned this Oct 21, 2024
@StephanTLavavej
Copy link
Member

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

@StephanTLavavej StephanTLavavej merged commit a5d66bd into microsoft:main Oct 21, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for this significant improvement - and congratulations on your first microsoft/STL commit! 🎉 😻 🚀

I've filed a followup issue because we need to "triple-mirror" this change into the VS repo in order for it to actually take effect in the IDE.

@Nerixyz Nerixyz deleted the feat/chrono-timepoint-natvis branch October 22, 2024 05:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
visualizer How the VS debugger displays STL types
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants