User32 extensions to support Keyboard related functionality. #1063

kevemueller · 2019-02-04T08:59:11Z

Extend User32 signatures. Introduce utility function for convenient use
of LoadString and enum for all defined Win32 VK.

Extend User32 signatures. Introduce utility function for convenient use of LoadString and enum for all defined Win32 VK.

matthiasblaesing

I left some comments inline. Please also add an entry to CHANGES.md.

matthiasblaesing · 2019-02-09T17:29:20Z

contrib/platform/src/com/sun/jna/platform/win32/Win32VK.java

+	 */
+	public final int code;
+	/** This VK may be used to map to a unicode codepoint in a Keyboard Layout. */
+	public final boolean mappable;


Where does this info come from?

This is based on own investigation. I understand that this might be borderline to include here, but it is such a useful bit of information. I can refactor the code to leave it out if you consider it out-of-policy.

I gave this a second thought and went for a feasible alternative. I have taken out mappable member and introduced User32Util.WIN32VK_MAPPABLE, a set of the mappable virtual keys. This makes the enum clean (based purely on header definitions) and keeps the information in a concise way as a useful utility.

matthiasblaesing · 2019-02-09T17:31:49Z

contrib/platform/src/com/sun/jna/platform/win32/Win32VK.java

+	 * @return the VK enum instance.
+	 */
+	public static Win32VK fromValue(final int code) {
+		if (code < 0 || code > 0xFF) {


Any reason to special case this? The fall through to line 300 should catch it.

matthiasblaesing · 2019-02-09T17:32:32Z

contrib/platform/src/com/sun/jna/platform/win32/Win32VK.java

+	 * Virtual Keys, Standard Set
+	 */
+	VK_LBUTTON(0x01, false), VK_RBUTTON(0x02, false), VK_CANCEL(0x03, true), VK_MBUTTON(0x04, false), /*
+																										 * NOT


lines 15-19 look strange (indent?)

matthiasblaesing · 2019-02-09T17:33:21Z

contrib/platform/src/com/sun/jna/platform/win32/Win32VK.java

+	/*
+	 * Virtual Keys, Standard Set
+	 */
+	VK_LBUTTON(0x01, false), VK_RBUTTON(0x02, false), VK_CANCEL(0x03, true), VK_MBUTTON(0x04, false), /*


Please define one enum per line - its difficult to see where one entry ends and the next starts.

matthiasblaesing · 2019-02-09T17:46:32Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	/**
+	 * Handle to a input locale identifier (formerly called keyboard layout handle).
+	 */
+	public static class HKL extends HANDLE {


The definition of HKL should be in the file matching its header - that would be WTypes.java. I'm not sure if HANDLE is the correct super class. The C definition is typedef void *HKL. There is also the question: where does the information about the internal structure (embedded language identifier and device handle) come from?

HKL is defined as DECLARE_HANDLE(HKL); in minwindef.h
So it should be defined same way as e.g. HINSTANCE in the same file.
I have moved it to WinDef.java.

The internal structure is described in the Javadoc of GetKeyboardLayout, which is taken from official docs.

matthiasblaesing · 2019-02-09T17:55:39Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	 *         If the function finds no key that translates to the passed character
+	 *         code, both the low-order and high-order bytes contain -1.
+	 */
+	short VkKeyScanEx(char ch, HKL dwhkl);


this is problematic - there are to definitions for this function:
https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-vkkeyscanexw
https://docs.microsoft.com/en-us/windows/desktop/api/winuser/nf-winuser-vkkeyscanexa
At this point I would consider to explicitly map

short VkKeyScanExW(char ch, HKL dwhkl); short VkKeyScanExA(byte ch, HKL dwhkl);

matthiasblaesing · 2019-02-09T17:57:13Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	 *         character value, depending on the value of uCode and uMapType. If
+	 *         there is no translation, the return value is zero.
+	 */
+	UINT MapVirtualKeyEx(UINT uCode, UINT uMapType, HKL dwhkl);


UINT is defined to be 32 bit wide (https://docs.microsoft.com/en-us/windows/desktop/winprog/windows-data-types) it might be substituted with an int.

UINT is extensively used in User32. I have it there for consistency with the remainder of the code.

User32 is old, the current calling parameters will not change to keep the API stable. If there is a good reason to use an UINT I'm good with it. One thing UINT can represent, that int can't is the unsignedness. So if the values need to be ordered a UINT might be better.

matthiasblaesing · 2019-02-09T18:04:28Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	 *         extended error information, call GetLastError.
+	 * 
+	 */
+	int LoadString(HINSTANCE hInstance, UINT uID, LPSTR lpBuffer, int cchBufferMax);


'lpBufferisLPTSTRand thus changes depending whether or not the unicode version is called. I would use aPointer` and add a convinience function to decode it. Have a look at this:
com.sun.jna.platform.win32.Kernel32Util.expandEnvironmentStrings(String)
there memory is allocated based on characters and read based on the unicode settings.

Pointer - OK.
There is already a convenience function using the more common way to call this function.

contrib/platform/src/com/sun/jna/platform/win32/User32.java

matthiasblaesing · 2019-02-09T18:10:20Z

contrib/platform/src/com/sun/jna/platform/win32/User32Util.java

+    		if (0 == x) {
+    			throw new Win32Exception(Kernel32.INSTANCE.GetLastError());
+    		}
+    		return new String(lpBuffer.getValue().getCharArray(0, x));


This looks suspicious. As written above, you could get a non unicde value, than you are interested in the byte array. And the Native#toString functions.

Pointer.getString() and Pointer.getWideString() cannot be used as the returned buffer might not be null terminated.
I am now distinguishing between Unicode and non-unicode.

Conflicts: CHANGES.md

matthiasblaesing

Thank you for your work. In general this looks good. I left a few comments inline. One think I'd like to see before merging are unittests for the new functions. I learned the hard way, that they help.

matthiasblaesing · 2019-02-13T18:18:09Z

contrib/platform/src/com/sun/jna/platform/win32/WinUser.java

@@ -2001,4 +2003,83 @@ public String toString() {
     * or 22 kHz PCM.
     */
    public int CF_WAVE = 12;
+


Please correct the indention. There is not "one right" indention style for JNA, but in most cases 4 spaces per indention level is correct. Tabs are normally not used.

matthiasblaesing · 2019-02-13T18:22:26Z

contrib/platform/src/com/sun/jna/platform/win32/WinUser.java

+	/**
+	 * Bitmask for the SHIFT key modifier.
+	 */
+	int MODIFIER_SHIFT_MASK = 1;


Where do the "MODIFIER_*" constants from? I could not find in my headers.

They are in the documentation of the function. I have extracted them as constants.

matthiasblaesing · 2019-02-13T18:29:32Z

contrib/platform/src/com/sun/jna/platform/win32/WinDef.java

+	/**
+	 * Handle to a input locale identifier (formerly called keyboard layout handle).
+	 */
+	public static class HKL extends HANDLE {


Please reindent (4 spaces per ident level)

matthiasblaesing · 2019-02-13T18:35:12Z

contrib/platform/src/com/sun/jna/platform/win32/User32Util.java

+        /**
+         * Set of {@link Win32VK} members that can be mapped to a UniCode code point via a keyboard layout.
+         */
+		public static final EnumSet<Win32VK> WIN32VK_MAPPABLE = EnumSet.of(Win32VK.VK_BACK, Win32VK.VK_TAB, Win32VK.VK_CLEAR, Win32VK.VK_RETURN,


If I'm not mistaken the WIN32VK_MAPPABLE property is derived, as is the introducedVersion in the Win32VK enum. You pulled one out. I have no strong feelings either way, if you want to reintegrate the mappable property from the first revision, I would be ok with it.

I like the mappable property like this. Very convenvient to use and clearly distinguished header values vs. "other known" values.
introducedVersion is part of the header (IFDEF), so it should be in the enum.

matthiasblaesing · 2019-02-13T18:48:13Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	 *         character value, depending on the value of uCode and uMapType. If
+	 *         there is no translation, the return value is zero.
+	 */
+	UINT MapVirtualKeyEx(UINT uCode, UINT uMapType, HKL dwhkl);


User32 is old, the current calling parameters will not change to keep the API stable. If there is a good reason to use an UINT I'm good with it. One thing UINT can represent, that int can't is the unsignedness. So if the values need to be ordered a UINT might be better.

matthiasblaesing · 2019-02-13T18:49:25Z

contrib/platform/src/com/sun/jna/platform/win32/User32.java

+	 *         the high word contains a device handle to the physical layout of the
+	 *         keyboard.
+	 */
+	HKL GetKeyboardLayout(DWORD idThread);


See my comment below for my reasoning (comment on UINT)

kevemueller · 2019-02-20T15:32:57Z

Hi @matthiasblaesing,

I have added the tests and also applied some minor cosmetics.

I have not found a source formatting definition. Please create one and then the source base can be reformatted in a tidy matter. I got rid of the tabs.

Hope this can be closed now.
Cheers,

Keve

matthiasblaesing · 2019-02-20T20:14:23Z

I added some more changes - see here: https://github.com/matthiasblaesing/jna/commits/pr-1063
Please see if that works for you and if you agree. If so, I would squash all commits, leave you as the author and merge to master.

kevemueller · 2019-02-21T13:08:51Z

Looked at your changes. All fine with me. Indeed the non-English locale was a good one...
Fully agree, esp. to the squashing.

Cheers,
Keve

matthiasblaesing · 2019-02-21T19:40:39Z

Merged as 07adbff
Thank you for your work.

Keve Müller added 4 commits February 4, 2019 09:57

User32 extensions to support Keyboard related functionality.

01ad2f0

Extend User32 signatures. Introduce utility function for convenient use of LoadString and enum for all defined Win32 VK.

ASCII

ff30ee5

...

291d6ad

Missed one JavaDoc.

1863841

matthiasblaesing requested changes Feb 9, 2019

View reviewed changes

Keve Müller added 4 commits February 10, 2019 18:27

Changes proposed by initial review from matthiasblaesing

909cd2e

Merge branch 'master' into user32keys

00243ec

Conflicts: CHANGES.md

Refactored Win32VK.mappable into a Set.

dcf3b44

Expose EnumSet so user can do complementOf.

a50e391

matthiasblaesing reviewed Feb 13, 2019

View reviewed changes

Keve Müller added 2 commits February 20, 2019 16:21

Added tests

9829da1

Merge branch 'master' into user32keys

3e24ba9

Keve Müller added 3 commits February 20, 2019 16:39

relaxed type mapping where possible

dbac844

spaces

3b058f1

...

21022bb

matthiasblaesing closed this Feb 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User32 extensions to support Keyboard related functionality. #1063

User32 extensions to support Keyboard related functionality. #1063

kevemueller commented Feb 4, 2019

matthiasblaesing left a comment

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

kevemueller Feb 11, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 13, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing Feb 9, 2019

kevemueller Feb 10, 2019

matthiasblaesing left a comment

matthiasblaesing Feb 13, 2019

matthiasblaesing Feb 13, 2019

kevemueller Feb 20, 2019

matthiasblaesing Feb 13, 2019

matthiasblaesing Feb 13, 2019

kevemueller Feb 20, 2019

matthiasblaesing Feb 13, 2019

matthiasblaesing Feb 13, 2019

kevemueller commented Feb 20, 2019 •

edited

Loading

matthiasblaesing commented Feb 20, 2019

kevemueller commented Feb 21, 2019

matthiasblaesing commented Feb 21, 2019

User32 extensions to support Keyboard related functionality. #1063

User32 extensions to support Keyboard related functionality. #1063

Conversation

kevemueller commented Feb 4, 2019

matthiasblaesing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthiasblaesing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevemueller commented Feb 20, 2019 • edited Loading

matthiasblaesing commented Feb 20, 2019

kevemueller commented Feb 21, 2019

matthiasblaesing commented Feb 21, 2019

kevemueller commented Feb 20, 2019 •

edited

Loading