-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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 LowLevelMouseProc #1263
Add LowLevelMouseProc #1263
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment inline. Is is possible to add a test case to WinUserTest User32Test that exercises this function?
Where is WinUserTest located? I can't find it in contrib/platform/test/com/sun/jna/platform/win32/ |
I meant User32Test, sorry, although it's a question of whether it's possible; If the test relies on user interaction that may not be the case. However, the equivalent keyboard method is exercised in the KeyHook.java demo class. It would be useful to have a similar "MouseHook" class where it can be manually demonstrated/tested. (Which I belatedly see you commented on in your very first post! So the answer is, yes please!) |
Done. |
This looks good to me. Anything missed, @matthiasblaesing ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would integrate the mouse sample hook with the sample for the keyboard hook. From a user perspective they are very similar and I think they could be savely combined. The sample might then be better called w32windowhooks
.
I left one IMHO critical comment inline, as this sets a bad example (callbacks must be kept strongly referened).
It would be great if you could try to keep more of KeyHook untouched. If less of the source is changed, git should be able to pickup the rename. Existing functionality like being able to easily exit would be available then. |
So I'll just add this to the KeyHook? |
Thank you - merged via e502ea5. |
Should I make a demo like here?