8000 fix: GetVariantDate return with milliseconds by huskar-t · Pull Request #252 · go-ole/go-ole · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

fix: GetVariantDate return with milliseconds #252

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 30, 2023
Merged

fix: GetVariantDate return with milliseconds #252

merged 2 commits into from
Oct 30, 2023

Conversation

huskar-t
Copy link
Contributor

the milliseconds value appears as zero or is ignored when use the Microsoft Automation function VariantTimeToSystemTime.I found this as reference https://www.codeproject.com/Articles/17576/SystemTime-to-VariantTime-with-Milliseconds . So I add milliseconds value to the GetVariantDate method.I modified and tested on amd64 and 386 architectures. At the same time, I doubt whether it can be used on arm architecture golang/go#62583
@mattn Looking forward to your reply

@huskar-t
Copy link
Contributor Author
huskar-t commented Oct 24, 2023 •
edited
Loading

@jacobsantos @mattn Please take a look at this problem, it's urgent for me

@jacobsantos
Copy link
Member

@jacobsantos @mattn Please take a look at this problem, it's urgent for me

@huskar-t
I will need to research this a bit more. I don't know if we have unit tests for datetime. The PR code appears complicated and I am not sure if that is what is intended or necessary. I will need to research this again to be certain. Probably just need to expand the COM server and add test cases for covering this use case.

@huskar-t
Copy link
Contributor Author

@jacobsantos @mattn Please take a look at this problem, it's urgent for me

@huskar-t I will need to research this a bit more. I don't know if we have unit tests for datetime. The PR code appears complicated and I am not sure if that is what is intended or necessary. I will need to research this again to be certain. Probably just need to expand the COM server and add test cases for covering this use case.

I have a question about why int(st.Milliseconds/1000) is used as the nanosecond value

@jacobsantos
Copy link
Member

I have a question about why int(st.Milliseconds/1000) is used as the nanosecond value

It is likely an artifact from when it was first developed. COM and OLE is from a time when there were a lot of implementations for datetime. So it is likely, "this makes sense to do it this way" vs "this is the way it is in reality."

Without unit tests, it would not be possible to verify.

@huskar-t
Copy link
Contributor Author

@jacobsantos Since it's late at night for me here, I'll finish the unit test as soon as I can tomorrow

@jacobsantos
Copy link
Member

@jacobsantos Since it's late at night for me here, I'll finish the unit test as soon as I can tomorrow

Thanks! I was going to do the work last week and I totally forgot. It would be awesome of you and help push this through.

@huskar-t
A49C
Copy link
Contributor Author

@jacobsantos I have added some simple tests

@jacobsantos jacobsantos merged commit 12e70eb into go-ole:master Oct 30, 2023
@huskar-t
Copy link
Contributor Author
huskar-t commented Nov 2, 2023

@jacobsantos When will the new version be released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@huskar-t @jacobsantos
0