On Fri, 7 Apr 2023 17:19:56 GMT, Martin Fox <[email protected]> wrote:
>> When processing a `WM_CHAR` event on an OEM key (punctuation, symbol, dead
>> key) the glass code will dynamically query the key's unshifted character to
>> determine the Java code to assign to it. This is necessary since the
>> relationship between OEM key codes and the characters they generate varies
>> from layout to layout.
>>
>> The Robot implementation was consulting a table which assumed a fixed
>> relationship between Java codes and Windows key codes even for the OEM keys.
>> The table was also missing entries for any Java code not on a US QWERTY
>> layout, like PLUS.
>>
>> In this PR if we don't find the Java code in the table or if it maps to an
>> OEM key (which may be wrong) we sweep through all the OEM keys looking for
>> the matching Java code.
>
> Martin Fox has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated license header copyrights
The code changes look fine to me. I do have two minor points about code style.
* Please enclose all single-line targets of `if`, etc., with curly braces.
* I also recommend (but do not insist), that the opening curly brace the target
block of `if`, etc., be on the same line as the `if`.
We aren't quite as rigorous on code style in native code, but for code that
isn't third-party, consistency is a good thing.
modules/javafx.graphics/src/main/native-glass/win/KeyTable.cpp line 210:
> 208: {
> 209: if (oemKeys[i] == vkey)
> 210: return true;
Please enclose this in curly braces (or move the `return true;` to be on the
same line as the `if`).
modules/javafx.graphics/src/main/native-glass/win/KeyTable.cpp line 287:
> 285:
> 286: if (jkey == com_sun_glass_events_KeyEvent_VK_UNDEFINED)
> 287: return;
Same.
-------------
PR Review: https://git.openjdk.org/jfx/pull/702#pullrequestreview-1376569816
PR Review Comment: https://git.openjdk.org/jfx/pull/702#discussion_r1160954017
PR Review Comment: https://git.openjdk.org/jfx/pull/702#discussion_r1160955289
