Skip to content
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

New Feature to map Ctrl to Esc on Release #113

Merged
merged 9 commits into from May 14, 2020
Merged

New Feature to map Ctrl to Esc on Release #113

merged 9 commits into from May 14, 2020

Conversation

ghost
Copy link

@ghost ghost commented May 11, 2020

This adds a new app setting to map ctrl to esc on release. That is if ctrl (either physical or mapped by iVim/iOS) is pressed and released without any other keys pressed, it'll act as an escape key.

bbales added 4 commits May 4, 2020 14:28
Track caps-lock state separately from alphaShift.
Didn't realize "none" was acting a bit different when I tested last. It
was treating caps-lock as both ctrl and the normal caps-lock. That is,
you could press caps-lock & h at the same time to delete a letter and
press caps-lock. Should be working now.
Adds option to make ctrl act as esc if released without pressing any
other keys. (If other keys are pressed, it'll continue to function as a
regular ctrl modifier.)
This supports mapping caps-lock to ctrl (either via iVim or system
settings). So if this new setting is turned on, both caps-lock
can also function as both ctrl and esc.

note: only tested on iPad and in combination with system-settings and
iVim's caps-lock settings.

Tests:
Editted text while randomly pressing and holding physical caps-lock or ctrl keys.
- When neither iVim nor System is changing Caps-Lock or Ctrl behavior
- When iVim's Caps-Lock is mapped to Ctrl
- When iVim's Caps-Lock is mapped to Esc
- When System's Caps-Lock is mapped to Ctrl
- When iVim's Caps-Lock is mapped to Ctrl and Ctrl-to-Esc-on-release is
  turned on
- When System's Caps-Lock is mapped to Ctrl and iVim's
  Ctrl-to-Esc-on-release setting is turned on
- When both iVim and System are mapping Caps-Lock to Ctrl and iVim's
  Ctrl-to-Esc-on-release setting is turned on
@terrychou
Copy link
Owner

The reason I said "we can even escape from the dependency on iOS 13.4" was that I thought the parts like presses.first?.key.map(keyPressed) were not necessary. Because the pressesXXX methods were introduced into iOS since version 9.0 and only the UIKey type requires 13.4.

What is the purpose of these pressesXXX override there? If we don't need to fix the "failure of mapping caps lock to ctrl in iOS 13", can we get rid of them completely?

@ghost
Copy link
Author

ghost commented May 12, 2020 via email

@terrychou
Copy link
Owner

I believe that if we register to capture only the ctrl (as the line VimViewController.keyCommand(input: "", modifierFlags: .control) does), we can get the event "ctrl key was pressed without any other key" right in method keyCommandTriggered. Can we do the esc injection there without the help from pressesXXX methods?

@ghost
Copy link
Author

ghost commented May 12, 2020 via email

@terrychou
Copy link
Owner

I see. How about we record that "only ctrl pressed" in keyCommandTriggered then we only need to decide to do the injection according to whether this record exists or not in pressesXXX methods? In this case, maybe parts like presses.first?.key.map(keyPressed) are not really necessary.

@ghost
Copy link
Author

ghost commented May 12, 2020 via email

@terrychou
Copy link
Owner

Let's get rid of the "fix of mapping caps lock to ctrl in iOS 13.4", because users would choose the system's native mapping over this, and our code would be lighter.

Also, I see you have changed the development version to 13.4. Can you please change it back to 10.0, so that I could do the merge.

@ghost
Copy link
Author

ghost commented May 12, 2020 via email

@ghost
Copy link
Author

ghost commented May 12, 2020

Added work around for iOS 10.0. Though, I haven't tested it on any devices with an iOS older than 13.4.

@ghost
Copy link
Author

ghost commented May 13, 2020

So these changes changes come with a bug due to the lack of using iOS 13.4.

Repo:

  • In insert mode
  • Press and hold h
  • Press and hold ctrl
  • Release h
  • A virtual esc key press will be injected, even before releasing ctrl

keyCommandTriggered will believe only the ctrl key is being pressed when you press ctrl (even though h is still be held). Then when you release h, it can't tell which key was released so it has to assume ctrl was released and thus injects a virtual esc.

So if someone pressing ctrl with one hand too quickly before releasing all other keys, they're going to encounter this bug. I encountered this several times even just casually using vim.

@ghost
Copy link
Author

ghost commented May 13, 2020

I recommend just upgrading to iOS 13.4, so this and any other edge-cases can be avoided.

@ghost
Copy link
Author

ghost commented May 13, 2020

Or at least condition the feature to only show in iOS 13.4

@terrychou
Copy link
Owner

If the bug happens this frequently, it is not usable. Then unfortunately it looks like we can only make this feature available to 13.4 and above. Can you please add the code to condition this feature only to iOS 13.4 and above? Then I will do the merge, and add related documentation on it.

…had bugs) + Cleaner implementation

Repo:
- In insert mode
- Press and hold h
- Press and hold ctrl
- Release h

A virtual esc key press will be injected, even before releasing
ctrleyCommandTriggered will believe only the ctrl key is being pressed
when you press ctrl (even though h is still be held). Then when you
release h, it can't tell which key was released so it has to assume ctrl
was released and thus injects a virtual esc.

So if someone pressing ctrl with one hand too quickly before releasing
all other keys, they're going to encounter this bug. I encountered this
several times even just casually using vim.

merge feature
@ghost
Copy link
Author

ghost commented May 13, 2020

Feature's now conditioned for versions 13.4 and up. (Though, I'm not sure how to test it on lower versions.)

Also, cleaned up the implementation a bit.

@terrychou terrychou merged commit fe7a03e into terrychou:master May 14, 2020
@terrychou
Copy link
Owner

I have tested it both on iOS 13.4 and 12.4.5 (not supposed to work), and it worked like a charm. It will be included into the TestFlight testing before its final release.

@ghost ghost deleted the ctrltoesc branch May 15, 2020 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant