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

Partial implementation of virtualedit config #242

Merged
merged 7 commits into from
Sep 17, 2020

Conversation

i-e-b
Copy link
Contributor

@i-e-b i-e-b commented Jun 26, 2020

This does not support all config settings,
but does add the 'onemore' option.

This partly addresses https://youtrack.jetbrains.com/issue/VIM-844

@AlexPl292
Copy link
Member

Yep, thanks for the contribution. We have a pre-release freeze, so I'll check the PR right after the release.

Are you going to support other settings, or you're interested in this particular one? The both answers are perfectly fine, but I just want to understand your goals :)

@i-e-b
Copy link
Contributor Author

i-e-b commented Jun 26, 2020

My primary purpose was to support onemore, mainly as it makes mouse-based copy-and-paste actions much more intuitive.
If I have time later, I might try the other settings, but there is an interaction between Vim settings and Idea virtual space settings I'd have to work out.

@i-e-b
Copy link
Contributor Author

i-e-b commented Jul 14, 2020

@AlexPl292 : Updated to fix conflicts.

@AlexPl292
Copy link
Member

HI! I've just checked the PR. Unfortunately, it looks like it would be a bit more complicated. Here are some inconsistencies I was able to find:

  • D command after the end of the line
  • D command in the middle of the line
  • $ command and moving to a shorter line.

2020-07-24 10 45 01

Could you please check these cases and write some tests to prove the implementation?

Also, it looks like there are some unnecessary files in the PR.
But overall, the implementation looks good. Seems like it can be merged after fixing these small issues.

@i-e-b
Copy link
Contributor Author

i-e-b commented Jul 27, 2020

Thanks for the detailed feedback. I'll try to correct those issues as soon as possible.

@i-e-b i-e-b force-pushed the master branch 7 times, most recently from 30ade61 to 702ff40 Compare July 29, 2020 12:22
@i-e-b
Copy link
Contributor Author

i-e-b commented Jul 29, 2020

Hi @AlexPl292 .
The PR should be cleaned up and have tests that match your description above.

It turns out the interaction between $ and visual mode are complex, and IdeaVim doesn't currently match Vim 8.x
Rather than try to re-work everything, I've tried to end up with the most reasonable behaviour for onemore that doesn't break existing IdeaVim behaviour.

This does not support all config settings,
but does add the 'onemore' option.

This partly addresses https://youtrack.jetbrains.com/issue/VIM-844
@i-e-b
Copy link
Contributor Author

i-e-b commented Aug 26, 2020

Updated against upstream. As far as I'm aware, there are no other changes required?

@AlexPl292 AlexPl292 merged commit 5bf2818 into JetBrains:master Sep 17, 2020
@AlexPl292
Copy link
Member

Hi! I was able to perform some more aggressive refactorings that allowed me to reach the desired behaviour without additional workarounds. I hope this won't give us any other difficulties.
Thank you for your contribution! This was really a big change!

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