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

Reset operator-pending commands on escape #237

Merged
merged 1 commit into from Jul 9, 2020
Merged

Reset operator-pending commands on escape #237

merged 1 commit into from Jul 9, 2020

Conversation

adriafarres
Copy link

Suppose we have the text "|Hello, world", with | being the position of
the cursor. Pressing d<Esc>dw simply moves the cursor on top of the
comma instead of leaving the text as ", world".

This fixes issue VIM-1421.

I'm creating the pull request again, since on rebasing I apparently messed up the history.

@AlexPl292
Copy link
Member

Hi @adriafarres !
Thank you for your attention to the project!
At the moment it looks like your PR doesn't handle some cases where <Esc> is mapped to any other key, or pressing <C-[>. Could you take a look at handleEditorReset and isEditorReset functions and it's usages? It might be a better place for this fix, what do you think?

Suppose we have the text "|Hello, world", with | being the position of
the cursor. Pressing `d<Esc>dw` simply moves the cursor on top of the
comma instead of leaving the text as ", world".

This fixes issue VIM-1421.
@adriafarres
Copy link
Author

You are absolutely right, Alex. In fact, this case was already being handled and I missed it. My apologies. Moving it outside of the condition seems to fix it. Let me know if there's a better way to handle this or if you would like me to add more tests.

@AlexPl292
Copy link
Member

This might be a proper solution. And yep, I would appreciate if you could add more test cases to prove the change. I think, also <C-[>, different commands and mappings like map <C-D> <Esc> should be tested as well.

@AlexPl292
Copy link
Member

We've got more tests here: #241
Thank you for the contribution! :)

@AlexPl292 AlexPl292 merged commit e222294 into JetBrains:master Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants