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(keys): refactor retrigger mechanism #428

Merged
merged 4 commits into from
Feb 7, 2023

Conversation

MurdeRM3L0DY
Copy link
Contributor

should fix #286.

@folke
Copy link
Owner

folke commented Jan 19, 2023

I did already fix that, no?

I use mini.ai myself with those keybiondings and it works as expected

@MurdeRM3L0DY
Copy link
Contributor Author

yes, you fixed the main problem. However consider this case:

  1. you have the string "hello"
  2. if you try to ciw with the cursor on the character h, you'll actually end up performing the action on the previous character " which happens to be a word in vim

I suspect this happens because c operator enters insert mode so when you retrigger to re-enter normal mode, the cursor shifts backwards by one position.

@folke
Copy link
Owner

folke commented Jan 20, 2023

Thank you for the clarification. Will need to check this PR with some issues from the past related to keymaps.

I'm also not that sure of the vim.schedule in there

@MurdeRM3L0DY
Copy link
Contributor Author

MurdeRM3L0DY commented Jan 20, 2023

Yeah. I agree with the sentiment about using vim.schedule.

I've got another solution via this branch which uses expr and remap to retrigger the original keymap.

Edit: I've figured out a better solution for the retrigger logic

we can just eval `"<Ignore>" .. lhs` to retrigger the mapping
@folke
Copy link
Owner

folke commented Jan 21, 2023

I implemented the solution you propose a while back, but it failed some issues people had in the past. Although, I did not add that <ignore>.

What I'll do first, I'll add a bunch of unit tests for those issues I'm talking about, so we have something to test this with. Will do so on Monday.

@MurdeRM3L0DY
Copy link
Contributor Author

Understood. From my various "tests" it works pretty well:

  • loading multiple plugins with one keymap. e.g gzaiw[char] loads mini.surround then mini.ai for me
  • count works as expected

@mathjiajia
Copy link

yes, you fixed the main problem. However consider this case:

  1. you have the string "hello"
  2. if you try to ciw with the cursor on the character h, you'll actually end up performing the action on the previous character " which happens to be a word in vim

I suspect this happens because c operator enters insert mode so when you retrigger to re-enter normal mode, the cursor shifts backwards by one position.

When will this pr be merged?
I also encountered this issue.

@folke
Copy link
Owner

folke commented Feb 6, 2023

I still need to check existing issues related to keymaps and make sure they still work as expected. If you want to help, add a comment to this PR linking to existing issues I should re-check with this PR

@MurdeRM3L0DY MurdeRM3L0DY changed the title fix keymap retrigger in operator mode fix(keys): refactor retrigger mechanism Feb 7, 2023
@MurdeRM3L0DY
Copy link
Contributor Author

AFAICT, these are the more relevant issues regarding the keys handler:

All of these seems to be fixed using neovim master but not on neovim stable (0.8.x). Particularly:

  • keys need to be pressed twice(sometimes)
    • Upon opening neovim, When I perform any(literally) action, subsequent registered keys need to be pressed twice to trigger
  • keys are pasted when triggering in visual mode.

@folke
Copy link
Owner

folke commented Feb 7, 2023

ok, thanks. I'll try to look into it later today. I do think the pending keys probably need to be added to your PR, but I'll see if it's needed when I encounter issues

@folke folke merged commit 4272d21 into folke:main Feb 7, 2023
@folke
Copy link
Owner

folke commented Feb 7, 2023

I checked all existing issues and these changes indeed seem to work for all of them. Thanks for fixing this!

@MurdeRM3L0DY
Copy link
Contributor Author

Happy to help! Though I did mention that there were some issues with neovim stable. Were you able to verify this?

@folke
Copy link
Owner

folke commented Feb 8, 2023

It all seemed to work for me on stable. What issue in particular didnt work on stable for you?

@folke
Copy link
Owner

folke commented Feb 8, 2023

This one seems to work on nightly, but not on stable. #511

Will look into it

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.

bug: operator-mode mappings don't get retriggered correctly the first time.
3 participants