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

fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys #1701

Merged

Conversation

sakurawald
Copy link
Contributor

@sakurawald sakurawald commented Dec 10, 2024

In pr: #1691, we flush the pending-keys for self-insert command in vi-insert-mode, so that we can define keymap like (define-key lem-vi-mode:*insert-keymap* "j k" 'lem-vi-mode/commands:vi-normal).

The effect of this define-key is:

  1. Press j k in vi-insert-mode will the lem-vi-mode/commands:vi-normal command.
  2. Press j <second-key> in vi-insert-mode will self-insert j and self-insert <second-key>.

However, in the vim editor, if the <second-key> is NOT print-able key, then it will quit the key pending state, and will not self-insert the <second-key>.

A case is: press j and Backspace.
This pr fix this edge-case.


NOTE:
This pr has some minor difference from the vim. The named-key is not identical to print-able-key, taken Tab for example.
I didn't add the function printable-key-sym-p or un-printable-key-sym-p, because it is hard to decide whether a key is print-able or not.

The difference is minor, and sufficient in daily use.

@sakurawald sakurawald changed the title fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys [DRAFT] fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys Dec 10, 2024
@sakurawald sakurawald changed the title [DRAFT] fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys Dec 10, 2024
@sakurawald
Copy link
Contributor Author

sakurawald commented Dec 10, 2024

The vi-pre-command-hook calles the pre-command-hook function.
However, i would like to write this feature in on-command-hook and using :around method-combination to do the effect. (The :around gives the possibility to cancel the self-insert j, if the <second-key> is un-printable-key.)

Screenshot_20241210_120359

@sakurawald sakurawald changed the title fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys [DRAFT] fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys Dec 10, 2024
@sakurawald
Copy link
Contributor Author

sakurawald commented Dec 10, 2024

Another joint to handle the pending-keys flushing for self-insert command is to use (defmethod execute :around (mode (command self-insert) argument), so that we can cancel the executing of self-insert command, if the <second-key> is un-printable-key.
However, this method only support to specify the major mode.


I think it's ready to be merged.

@sakurawald sakurawald changed the title [DRAFT] fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys fix: the self-insert command in vi-mode should not insert un-printable char while flushing the pending-keys Dec 10, 2024
@sakurawald
Copy link
Contributor Author

sakurawald commented Dec 10, 2024

Currently, for self-insert command in vi-mode, we flush the keys in (cdr this-command-keys) into the buffer. (We do this via the post-command-hook in vi-insert-mode)

Since the emacs-mode seldom use pure alpha letters to define-key. (Emacs define-keys using alpha-letter with modifier keys), and vi-mode does define some keys using pure alpha-letter like define jk in vi-insert-mode).
I impl the flushing function in post-command-hook of vi-mode, not the global executing :around for self-insert command.

@cxxxr cxxxr merged commit d574b01 into lem-project:main Dec 10, 2024
2 checks passed
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.

2 participants