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 textobjects for XML, HTML and jsx #11158

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

Conversation

Ansimorph
Copy link

@Ansimorph Ansimorph commented Jul 13, 2024

Building on the work done by @ff2400t this adds text-objects to select xml-like elements in html, jsx and xml files.

Fixes: #6682

The behaviour is consistent with other text-objects in helix. It is however not consistent with vim's tag textobject:

<p>
  <span> inner_span </span>
</p>

If the cursor is on <span> vit would select inner_span in vim.
In Helix mix will select <span> inner_span </span>
If the cursor is actually inside the tag, both solutions work the same.

I think despite the difference to vim this textobject is very useful and would love to see it in my daily work.

@Ansimorph Ansimorph force-pushed the html-tag-textobject branch 3 times, most recently from feb505b to a5b50d1 Compare July 13, 2024 16:47
@Ansimorph Ansimorph marked this pull request as ready for review July 13, 2024 16:47
@Ansimorph Ansimorph changed the title feat: Add textobjects for XML, HTML and jsx Add textobjects for XML, HTML and jsx Jul 13, 2024
@David-Else
Copy link
Contributor

David-Else commented Aug 27, 2024

Awesome work, thanks!

Can anything be done about HTML tags that don't have closing tags ("void elements" like <img> and <input>) causing the following issue? Try playing about with:

<div class="flex flex-col items-center">
  <input
    placeholder="Enter your email"
    type="email"
    name="EMAIL"
    class="required email mb-4 p-2 border border-gray-300 rounded"
    id="mce-EMAIL"
    required=""
    value=""
  />
  <input
    type="submit"
    name="subscribe"
    id="mc-embedded-subscribe"
    class="button bg-blue-500 hover:bg-blue-600 text-white rounded py-2 px-4"
    value="Subscribe"
  />
</div>

mix anywhere inside the <div>, including inside an <input> selects inside the div, max inside an <input> selects an <input> rather than around the <div> which seems strange compared to mix.


(jsx_element (jsx_opening_element) (_)* @xml_element.inside (jsx_closing_element))

(jsx_element) @xml_element.around
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add this to runtime/queries/_jsx/textobjects.scm instead so that both jsx and tsx can inherit it? The file doesn't exist yet, but I tried it locally and it seems to work.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, I've moved it

@david-crespo
Copy link
Contributor

Thank you so much for doing this, it will make a big difference for me.

Is this intended behavior? I found it surprising that from the same starting point, max would produce a smaller selection than mix. It's related to the example you give but with two children instead of one, which shows mix and max do not have the same behavior when the cursor is inside the tag.

Starting position

mix

The target of "inner" is the <div>.

max

The target of "around" is the <Button>.

@Ansimorph
Copy link
Author

@David-Else thanks for the feedback and @david-crespo thanks for your well illustrated point!

The current implementation is consistent with other textobjects. See for example the function textobject with nested functions below, where maf also creates a smaller selection than mif.

I completely agree that the behaviour of the vim tag-textobject is more intuitive, but changing this is not possible in the textobject definition itself AFAIK. We would have to change the way helix handles the textobjects and this is a change that requires more rust knowledge than I currently have and would change this PR from a pretty minimal enhancement into something that requires a lot more discussion with the core team. This is why I tried to keep the PR simple, because a more wide reaching PR for the same problem went nowhere in the past.

Starting Position

Bildschirmfoto 2024-08-28 um 16 01 07

mif

Bildschirmfoto 2024-08-28 um 16 02 13

maf

Bildschirmfoto 2024-08-28 um 16 02 51

@david-crespo
Copy link
Contributor

Wise! Cool with me, it’s a great start.

@David-Else
Copy link
Contributor

@Ansimorph OK, if what I described are not bugs then this is fantastic anyway, and I hope it gets merged ASAP.

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.

tag textobject for working with html markup
3 participants