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

feat: wrap selection in <tag>, change surrounding <tag> and delete surrounding <tag> #12055

Closed
wants to merge 39 commits into from

Conversation

nik-rev
Copy link
Contributor

@nik-rev nik-rev commented Nov 11, 2024

NOTE: 1 feature from this PR I've moved to #12254 due to this one becoming too big in scope


Asciinema recording showcase of this feature: https://asciinema.org/a/EVuPLsPJnpPVclGH7dJjhvnrq


This PR was inspired by #11158 which adds text objects for XML, HTML and JSX tags allowing us to:

  • select inside of a tag
  • select the entire tag

A popular request is to add the ability to rename tags: #966
It would also be useful to delete surrounding tag, and add a tag around a selection

This PR adds those capabilities by adding x under the surround operations, since #11158 also uses x.


Change surrounding tag

  1. mrx opens a prompt prefilled with the name tag surrounding the current selection (e.g. span).
  2. When the user presses enter, the value of the prompt replaces the type of the tag surrounding the selection (if the user writes div, it'll replace span -> div)

Delete surrounding tag

  1. mdx deletes the closest opening and closest tag to the cursor

Add surrounding tag

  1. msx opens a prompt
  2. When the user presses enter, the value of the prompt creates a tag around each selection of the same type (if the user writes div, it'll add a <div></div> around each selection)

Related to #966 and #6682

@nik-rev nik-rev changed the title feat: add tag around selection, change surrounding tag and delete surrounding tag feat: wrap selection in <tag>, change surrounding <tag> and delete surrounding <tag> Nov 11, 2024
@kirawi
Copy link
Member

kirawi commented Nov 11, 2024

I don't think the maintainers would accept this in core because it's language-specific. Not 100% sure though. They might make an exception since XML (in particular, HTML) is so ubiquitous. On the other hand, I also see them preferring to work on a snippet manager within Helix building upon #9801.

@gyreas
Copy link

gyreas commented Nov 11, 2024

I think a generalised one will be more practical, asin mic<tag>,</tag> like in Kakoune, which extends the single character mode

@nik-rev
Copy link
Contributor Author

nik-rev commented Nov 11, 2024

I don't think the maintainers would accept this in core because it's language-specific. Not 100% sure though. They might make an exception since XML (in particular, HTML) is so ubiquitous. On the other hand, I also see them preferring to work on a snippet manager within Helix building upon #9801.

I feel like it's still worth to add because, as you said - HTML is ubiquitous and it's extremely handy to have such useful tools embedded into the editor like surround selection with tag and delete it

For example we have text object support for functions, types etc right in the editor. These are also language agnostic, not all languages have types

So I think such generic utilities for working in tags in general will be fitting, but of course it's the maintainers' choice

@nik-rev
Copy link
Contributor Author

nik-rev commented Nov 11, 2024

I think a generalised one will be more practical, asin mic<tag>,</tag> like in Kakoune, which extends the single character mode

what do you mean by generalised? This PR doesn't depend on any sort of parser/language. I haven't used Kakoune

@nik-rev
Copy link
Contributor Author

nik-rev commented Nov 13, 2024

sidenote: The logic to delete surrounding tag and change surrounding tag ended up being significantly harder than I initially anticipated

I managed to get it working though! It took me quite a long time. I'll publish my changes tomorrow

It's super fun for me to learn Rust by implementing new features for a project which I love. I'll be happy if it gets merged, because I genuinely think this feature is really useful. but if it doesn't I'll also be happy :). I'm just doing this for fun!

Also I learned so much from this, I really fell in love with Rust. I started learning it about a week ago specifically because I wanted to add some features to this project which I myself wish it had: https://github.com/helix-editor/helix/pulls/nikitarevenco

@nik-rev
Copy link
Contributor Author

nik-rev commented Nov 13, 2024

Ok, so I just pushed all the changes and now all 3 functionalities work as expected!

@nik-rev nik-rev marked this pull request as ready for review November 13, 2024 21:29
@kacperwyczawski
Copy link

Good job, thanks!

@nik-rev nik-rev marked this pull request as draft November 22, 2024 06:48
@nik-rev
Copy link
Contributor Author

nik-rev commented Nov 22, 2024

Drafted since I realised there are several edge cases that the current implementation won't be able to cover, and there is some additional work that needs to be done

@nik-rev
Copy link
Contributor Author

nik-rev commented Dec 18, 2024

I'm going to close this since

  1. you can rename tag with space + h
  2. you can wrap selection with tag, see this comment
  3. it's too dependent on HTML and i dont really think this should be in core
  4. if it was added to core, the implementation would be completely different. In this PR I am brute searching text forward and backward. A better PR would use tree sitter queries

@nik-rev nik-rev closed this Dec 18, 2024
@nik-rev nik-rev deleted the feat/rename-surround-tag branch January 5, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants