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 DateTime operators to SYSTEMTIME #667

Closed
elachlan opened this issue Aug 25, 2022 · 2 comments · Fixed by #672
Closed

Add DateTime operators to SYSTEMTIME #667

elachlan opened this issue Aug 25, 2022 · 2 comments · Fixed by #672
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@elachlan
Copy link
Contributor

Issue came up in a PR(dotnet/winforms#7662) as apart of the winforms conversion effort.

https://github.com/dotnet/winforms/blob/74e08305a520abc8cf176350072784c6d8d98ddc/src/System.Windows.Forms.Primitives/src/Windows/Win32/Foundation/SYSTEMTIME.cs

dotnet/winforms#7468

@elachlan elachlan added the enhancement New feature or request label Aug 25, 2022
@AArnott AArnott added the help wanted Extra attention is needed label Aug 25, 2022
@AArnott
Copy link
Member

AArnott commented Aug 25, 2022

Thanks for suggesting this. I'm not sure an implicit conversion operator is appropriate for this, but if the conversion is lossless and fast/alloc free, it's probably fine. In your sample conversion operators, one of them drops the millisecond field. We wouldn't want to do that in our conversion operators. If WinForms wanted to drop to second resolution, that should be done in their own code. Perhaps with a partial struct on their side that defines a method like SYSTEMTIME ToSecondResolution() or something like that.

A template file for this type is probably the best way to add these conversion operators.

@elachlan
Copy link
Contributor Author

@fernandanavamoya can you let us know why you are stripping the millisecond field?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants