Skip to content

feat(windows): emit keystrokes for imx keydown/up actions 🥑#6137

Merged
rc-swag merged 87 commits intofeat/windows/imx-support-common-corefrom
feat/windows/6099-emit-keystrokes-from-imx
Jan 24, 2022
Merged

feat(windows): emit keystrokes for imx keydown/up actions 🥑#6137
rc-swag merged 87 commits intofeat/windows/imx-support-common-corefrom
feat/windows/6099-emit-keystrokes-from-imx

Conversation

@rc-swag
Copy link
Copy Markdown
Contributor

@rc-swag rc-swag commented Jan 21, 2022

Fixes #6099

Moving this testing to #5936 As that branch needs this work merged to proceed.

@keymanapp-test-bot skip

Needs CapsTest User Testing

SUITE_SIMPLIFIED_CHINESE:
This keyboard will display the IMX window as soon as a string that matches the pinyin for one or more characters are typed.
For all these tests install the Simplified Chinese Keyboard cs-pinyin.cmp

TEST_SIMPLIFIED_ENTER_KEY:
Using Notepad or equivalent.
Type h a n z i - The IMX window will appear and in the top left the letters hanzi will be present
Choose the 5th option
Expected result: 汉字变换
Type Enter
The cursor should be on a new line.
Type hanzi and press Enter
Expected Result:
汉字变换
汉子

mcdurdin and others added 30 commits November 23, 2021 10:11
Relates to #3621.

Initial draft of keyboard to test start-of-sentence detection.
Relates to #3621.

This adds basic compiler support for the `begin newContext` and
`begin postKeystroke` statements, along with the corresponding `gn` and
`gpk` entry points in the compiled JavaScript keyboard.

TODO: `readonly` semantics, usage constraints, `&layerChanged`.
Relates to #3621.

Adds basic support for `begin newContext`.
Relates to #3621.

This adds support for `begin postKeystroke` to KeymanWeb. This is now
close to final, apart from support for `&layerChanged`.
Relates to #3621.

Add support for `&layerChanged` system store. This store is set to `1`
before a `begin postKeystroke`, if the keystroke it follows resulted in
a layer change, either programatically through a keyboard rule, or
through a `nextlayer` property of the touched key.
Relates to #3621.

Adds `readonly` group support and verifies that keyboards meet
constraints around usage of these groups.

(Note: fixes test_valid.kps and test.bat which seemed to have some
invalid tests?)
Picked up in development of #5963.
…-overflow-1

fix(developer): kmdecomp string overflow
Co-authored-by: Darcy Wong <darcy_wong@sil.org>
Relates to #3621.

Adds `&layoutChanged`, `begin newcontext`, `begin postkeystroke` support
to IDE editor and keyword help.
Relates to #3620.

Allows keyboard developers to add a Caps Lock layer (called 'caps'),
which will be accessible through double-tap of Shift key, in an upcoming
PR.

Also adds test keyboards to exercise the functionality when it is
available.
Fixes #3620.

Implements the Caps Lock layer support and the double-tap gesture on the
shift key to access it.

The double-tap gesture has been implemented with a view to extension to
support other multi-tap gestures in the future. However, for now, it is
limited to supporting the Shift key, if and only if the keyboard
includes a Caps layer.

The reason for this v15 limitation is that multi-tap on regular keys
would involve either rewinding the previous keystroke (the first tap),
or forcing keyboard developers to consider 'rota' style rules in their
keyboards to support the multi-tap gestures, as we need to make sure
that the first tap is accepted and processed for immediate feedback.
This needs more design, to avoid unnecessary complexity in the keyboards
and/or the rewinding of the keystroke (even though that is conceptually
supported in Keyman Engine for Web already). Basically, we don't want to
constrain the way that a keyboard author may use the multi-tap gesture
by hard-coding the rewind, but neither do we want to make all multi-tap
gestures needlessly complex to author.

The shift key (and other modifiers, potentially in future) needs special
support for multi-tap as the key that is being tapped changes with the
layer change. This is currently managed through recognising `K_SHIFT` in
the key id.

I have tried to follow the `PendingGesture` pattern for multi-tap, and
the gesture itself supports a series of taps, not just a double-tap. The
maximum time to complete the tap series is 125msec * number-of-taps, so
for a double-tap is 250msec.

The changes to support a Caps Lock layer itself were minimal; just
adding the `text.KeyboardProcessor.getStateFromLayer` function and
calling it during `KeyEvent` construction. The remaining changes relate
to the multi-tap gesture.

Minor changes:
* I moved `constructNullKeyEvent` to `KeyEvent` in order to make it
  more accessible to other classes.
* The multi-tap gesture does not have a promise to complete, so that is
  now an optional member of the `PendingGesture` interface.
Removed the 'heuristic' test as it is probably not appropriate for
testing. Removed the mobile-specific platform from the touch layout as
it did not include a caps layer (and should have been identical to the
tablet one anyway).
…-ide-support

feat(developer): support for start-of-sentence in IDE 🍆
Relates to #3621.

Ensures that the `begin postKeystroke` entry point is called when the
user accepts a suggestion.
Co-authored-by: Eberhard Beilharz <ermshiperete@users.noreply.github.com>
Co-authored-by: rc-swag <58423624+rc-swag@users.noreply.github.com>
This checks in the crowdin strings for the locale `kr` for Kanuri.
Does not include macOS.

TODO: Check on adding ckl locale in XCode
This checks in the crowdin strings for the locale `mrt` for Marghi.
Does not include macOS.

TODO: Check on adding mrt locale in XCode
@rc-swag rc-swag marked this pull request as draft January 21, 2022 04:34
@rc-swag rc-swag marked this pull request as ready for review January 21, 2022 04:40
@rc-swag rc-swag added this to the A15S22 milestone Jan 21, 2022
@mcdurdin mcdurdin changed the title feat(windows): 6099 emit keystrokes for imx keydown/up actions 🥑 feat(windows): emit keystrokes for imx keydown/up actions 🥑 Jan 23, 2022
@mcdurdin mcdurdin modified the milestones: A15S22, B15S1 Jan 24, 2022
@mcdurdin
Copy link
Copy Markdown
Member

This is currently failing to build on most platforms. Some suggestions:

  • Test-14.0 (Keyman - Windows (Desktop/Developer))

    1. Build failure is fixed by a workaround for a bug in npm that has recently been merged to master.
    2. Update your local master
    3. git merge master
    4. Push
  • Test-14.0 (Keyman - Linux)

  • Test-14.0: Keyman Core - Linux (Common)

  • Test-14.0: Keyman Core - macOS (Common)

    All three of these have the same root cause.

    [15:36:38][Step 2/4] [3/57] Compiling C++ object 'src/kmnkbp0@sha/km_kbp_keyboard_api.cpp.o'.
    [15:36:38][Step 2/4] FAILED: src/kmnkbp0@sha/km_kbp_keyboard_api.cpp.o 
    [15:36:38][Step 2/4] c++  -Isrc/kmnkbp0@sha -Isrc -I../../../../common/core/desktop/src -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Werror -std=c++14 -O3 -fPIC -isystem ../../../../common/core/desktop/include -isystem include -DKMN_KBP_EXPORTING -Wall -Wctor-dtor-privacy -Wdouble-promotion -Wendif-labels -Wextra -Wno-unknown-pragmas -Wno-missing-field-initializers -Wnon-virtual-dtor -Wshadow -fvisibility=hidden -fvisibility-inlines-hidden -MD -MQ 'src/kmnkbp0@sha/km_kbp_keyboard_api.cpp.o' -MF 'src/kmnkbp0@sha/km_kbp_keyboard_api.cpp.o.d' -o 'src/kmnkbp0@sha/km_kbp_keyboard_api.cpp.o' -c ../../../../common/core/desktop/src/km_kbp_keyboard_api.cpp
    [15:36:38][Step 2/4] In file included from ../../../../common/core/desktop/src/km_kbp_keyboard_api.cpp:16:0:
    [15:36:38][Step 2/4] ../../../../common/core/desktop/src/kmx/kmx_processor.hpp:64:5: error: extra qualification ‘km::kbp::kmx_processor::’ on member ‘queue_action’ [-fpermissive]
    [15:36:38][Step 2/4]      kmx_processor::queue_action(
    [15:36:38][Step 2/4]      ^~~~~~~~~~~~~
    

@mcdurdin
Copy link
Copy Markdown
Member

This seems to extend feat/windows/imx-support-common-core but is not currently based on it, meaning we'd have to review a lot of code twice. Can you rebase?

@rc-swag rc-swag changed the base branch from master to feat/windows/imx-support-common-core January 24, 2022 00:42
@rc-swag
Copy link
Copy Markdown
Contributor Author

rc-swag commented Jan 24, 2022

This seems to extend feat/windows/imx-support-common-core but is not currently based on it, meaning we'd have to review a lot of code twice. Can you rebase?

It is cut from feat/windows/imx-support-common-core so should not need rebasing. It had the wrong target for merge it was for master instead of the feat/windows/imx-support-common-core. I will fix that.

@rc-swag
Copy link
Copy Markdown
Contributor Author

rc-swag commented Jan 24, 2022

This is currently failing to build on most platforms. Some suggestions:

  • Test-14.0 (Keyman - Windows (Desktop/Developer))

    1. Build failure is fixed by a workaround for a bug in npm that has recently been merged to master.
    2. Update your local master
    3. git merge master
    4. Push

I fixed the linux / macos comments. I will hold off merging master untill you have a chance to review as it will show the differences between feat/windows/imx-support-common-core and master which you don't want to see in the initial review

@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jan 24, 2022
@rc-swag rc-swag merged commit ad91e0e into feat/windows/imx-support-common-core Jan 24, 2022
@rc-swag rc-swag deleted the feat/windows/6099-emit-keystrokes-from-imx branch January 24, 2022 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(windows): Emit keystrokes from IMX dll call backs - keystroke processing optimization 🥑

6 participants