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

Add m textobject to select closest surround pair #1946

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

sudormrfbin
Copy link
Member

@sudormrfbin sudormrfbin commented Apr 3, 2022

m now acts as a wildcard surround textobject, so for example with this text

[abc (def) ghi]

If the cursor is on c, mim will select everything inside [], and if it's on e it will select inside (), without manually requiring either mi( or mi[.

@archseer
Copy link
Member

Looks like the test suite needs some adjustments?

@@ -66,6 +66,7 @@ Currently supported: `word`, `surround`, `function`, `class`, `parameter`.
| `w` | Word |
| `W` | WORD |
| `(`, `[`, `'`, etc | Specified surround pairs |
| `m` | Closest surround pair |
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than doing this, I think doing something more specific is better, like b for (, q for ' and similar, m is covers too much and the scope in too broad, user need to guess the context I believe to have this working the way they want.

Copy link
Member Author

@sudormrfbin sudormrfbin Apr 18, 2022

Choose a reason for hiding this comment

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

I've personally never needed this with vim-sandwich (from which I borrowed the m behavior), since it is supposed to be a catch-all -- that is it's sole purpose. If the closest pair isn't what I want I use the explicit form of mi(, etc instead of mim. The only drawback could be that we currently base the nearest pair matching off of the characters from the PAIRS constant which has some non-standard pairs:

pub const PAIRS: &[(char, char)] = &[
('(', ')'),
('[', ']'),
('{', '}'),
('<', '>'),
('«', '»'),
('「', '」'),
('(', ')'),
];

We could explore keys like b and q if there is interest in doing so, but I think m is a good first step.

@sudormrfbin sudormrfbin requested a review from archseer April 23, 2022 17:24
@archseer archseer merged commit 22ae1b9 into helix-editor:master Apr 29, 2022
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.

m should work with the nearest surround pair even if the cursor is not exactly on a surround character
3 participants