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 comment textobject for surround selection and navigation #1605

Merged

Conversation

EpocSquadron
Copy link
Contributor

This attempts to add a comment textobject to the surround matching logic. This seems like it would be an easy add, but I'm having trouble getting it to behave at all in Rust. In PHP @comment.inside works fine, but @comment.around does not.

I'm intending to support both @comment.inside and @comment.around, where the difference is that @comment.around should select adjacent line comments, so line comments like the following will match in total:

// This is some very long description of why some tricky logic works
// the way it does, split across lines rather than using block comments

.. whereas the .inside will only select one such line comment. Both methods are intended identical for block comments.

Once we figure this out it should partially address #1505.

@pickfire pickfire marked this pull request as draft January 31, 2022 09:33
@EpocSquadron
Copy link
Contributor Author

Pushed some changes that fix the issue with comment.inside not working for rust by rebinding to o. I also added comment textobjects to all languages that have some already defined. I figure we can put off the @comment.around textobject for now as that is seemingly a hard problem to solve.

We can discuss what the best letter to bind this textobject to is, but I think after that this is no longer a draft and ready for review.

@EpocSquadron
Copy link
Contributor Author

Waiting on #1611 and #1619 to be nerfed before I rework this to add comment.around, and add next/prev comment, respectively.

@EpocSquadron
Copy link
Contributor Author

Adjusted for #1619. Still waiting on #1611 for sensible @comment.around captures.

@EpocSquadron EpocSquadron force-pushed the epocsquadron/comment-textobject branch from a482330 to bcc9e6e Compare March 1, 2022 04:47
@EpocSquadron EpocSquadron marked this pull request as ready for review March 1, 2022 04:47
@EpocSquadron EpocSquadron changed the title Draft: Add comment textobject for surround selection Add comment textobject for surround selection and navigation Mar 1, 2022
Copy link
Member

@sudormrfbin sudormrfbin left a comment

Choose a reason for hiding this comment

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

This should also be mentioned in usage:

helix/book/src/usage.md

Lines 64 to 71 in a76e948

| Key after `mi` or `ma` | Textobject selected |
| --- | --- |
| `w` | Word |
| `W` | WORD |
| `(`, `[`, `'`, etc | Specified surround pairs |
| `f` | Function |
| `c` | Class |
| `p` | Parameter |

Looks good otherwise 🎉

This is to avoid conflicts with non-alphanumeric characters. There
are two reasons why this matters:

1. If `/` is bound, it won't be possible to `mi/` to purposefully match
inside of something like a regex, where you may actually mean to do so.
2. In some cases (e.g. Rust), the logic for surround selection seems to
miss that there is a binding for `/` and behave as in (1) instead.

`o` is chosen because it is a vowel inside "comment", although it could
easily be changed to `t` or even `e` if we think that is less likely to
conflict with future desired textobject bindings.
There is open question on how to match an entire group of adjacent
(comment) nodes, as each sub-comment also gets matched in the capture,
effectively negating the group selection. Until we can figure that out,
we can't define any sensible `@comment.around` rules in any language.
@sudormrfbin sudormrfbin merged commit 9bfb0ca into helix-editor:master Mar 6, 2022
@EpocSquadron EpocSquadron deleted the epocsquadron/comment-textobject branch March 6, 2022 13:42
Comment on lines +273 to +274
| `]o` | Go to next comment (**TS**) | `goto_next_comment` |
| `[o` | Go to previous comment (**TS**) | `goto_prev_comment` |
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem intuitive for me, I think ]# might be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only problem with # is that it is a shifted key usually, making it harder to quickly use, and breaks the pattern of using a letter for the other textobjects. That said, maybe we open an issue for further discussion?

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.

3 participants