Skip to content

Surround fixup #6860

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

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Surround fixup #6860

wants to merge 7 commits into from

Conversation

sql-koala
Copy link
Contributor

@sql-koala sql-koala commented Jul 11, 2021

What this PR does / why we need it:
This deals with a few minor issues, that came up with surround rewrite.

Which issue(s) this PR fixes
fixes #6812
fixes #6743
fixes #6848
fixes #7003

Special notes for your reviewer:
I tested this, with the switch set, there is no map on cs and lagfree maps on c are possible.

@@ -240,6 +240,8 @@ class Configuration implements IConfiguration {

surround = true;

disableDefaultPluginMappings = false;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

about this switch: I want to do one switch for all cases (right now there is only surround. there might be coerce cr or exchange cx).
Not one per possible plugin; that would bloat the code and config.
the only real use case I see, is the few users with custom keyboard layouts. For that, mappings like cr or cx will be wrong same as cs.

@kino-ma
Copy link

kino-ma commented Dec 5, 2021

I'm looking for updates on this PR 👀

@sql-koala
Copy link
Contributor Author

Hi @J-Fields can we get this merged?
The surround plugin is "broken" only for a niche group of user (custom keyboard layouts like dvorak ...) but they have no way to fix it themself. Only disable surround entirely.

@sql-koala
Copy link
Contributor Author

Hi @J-Fields I changed the flag as you suggested.

Build now fails because "prettier" on Readme.me. Can you fix that? Should be simple.
I tried, but my local build setup is out of sync with this repo master.

The code itself builds and works.

@lionel-
Copy link

lionel- commented Oct 14, 2023

Any chance this could be merged? This would greatly help users of alternative keyboard layouts.

@stasnocap
Copy link

Please, merge this

@qdii
Copy link

qdii commented Nov 4, 2024

Hello, I'd like this merged as well

@CorentinDeBoisset
Copy link

I created this PR, with a green CI, so it could be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants