windwp / nvim-ts-autotag

Use treesitter to auto close and auto rename html tag
MIT License
1.62k stars 86 forks source link

fix: Fix bugs present in the latest version of neovim and treesitter #136

Closed gungun974 closed 7 months ago

gungun974 commented 11 months ago

Introduction

Hoi! It's me again, the person who introduced the PR #116 that normally solves #94 (Anyway, it works well for me). Except it only works partially, let me explain.

Operations like closing tags for <div>Hoi</ properly transform to <div>Hoi</div> but I found recently when I was using TSX that self closing tags like <Img /> can't be written with my option.

There is actually a bug that when you type /, the code will think you are trying to close the tag (which is not the case of course) so it will write <Img/Img> which is awful.

So I initially decided to correct this but I discovered that the plugin was broken....

Yes, broken with the latest version of Treesitter.

So this PR will also address making the code work with the latest Treesitter version.

The changes

I can't exactly remember what all the issues were but here are some major ones I fixed with this PR:

Nested Identifiers don't close after inputting /

<Opt.Input></

Rename of Nested Identifiers or JSX fragments don't work or do some strange behavior

<div>
    <>
    </>
</div>

Gives when trying to rename the fragment:

<div>
    <p>
    </>
</p>

Sometimes closing after inputting / just doesn't work

<div></

You can't close self tags

<AppImage src="/hoi.png"

With the / that transforms to the awful original bug

<AppImage src="/hoi.png"/AppImage>

Conclusion

So after taking 6 hours to fix all of those problems, I am happy to present you with this PR that fixes every one of them.

I also updated the CI to the latest Neovim and Treesitter versions because some of the issues that were fixed were only caught by the CI with the latest version of Treesitter since Treesitter evolves and some tag identifiers change.

For example, the JSX fragment doesn't throw "Error" tags anymore so I need to directly check if the node is a Fragment in the buffer (which is an easy task).

If there is anything you see that could be improved, let me know ^^.

In any case, I love my days in the world of Neovim and I am even more honored to now be part of this community by helping this plugin once again.

PS: Sorry if this PR fixes more of my bugs that I introduced in #116 than anything else but that's life :eyes:

barrett-ruth commented 11 months ago

@gungun974 thanks for this awesome pr. I tested out the code and it works great.

One nitpick: jsx fragments still don't work with autopairs (typing out <><cr>). They also don't work when deleting a regular tag to try to turn it into a fragment in insert mode.

I'm also not sure if it's possible, but renaming tags while still being able to add attributes to the tag would be awesome. Like going from this:

<div>
</div>

to this:

<p x="a">
</p>

by pressing ciwpx="a" all at once.

Regardless, thanks for this epic PR. Hopefully windwp becomes active and merges it.

mmirus commented 11 months ago

Thank you for this! :partying_face: I'm going to install your branch until this gets merged.

tonyjara commented 11 months ago

This is exactly why I stopped using this. Thank you!

gungun974 commented 11 months ago

@gungun974 thanks for this awesome pr. I tested out the code and it works great.

One nitpick: jsx fragments still don't work with autopairs (typing out <><cr>). They also don't work when deleting a regular tag to try to turn it into a fragment in insert mode.

I'm also not sure if it's possible, but renaming tags while still being able to add attributes to the tag would be awesome. Like going from this:

<div>
</div>

to this:

<p x="a">
</p>

by pressing ciwpx="a" all at once.

Regardless, thanks for this epic PR. Hopefully windwp becomes active and merges it.

So personally I don't have all the time in the world available at the moment to customize my neovim even more. But I will try when I can to look if I can add this to this PR.

gungun974 commented 11 months ago

This is exactly why I stopped using this. Thank you!

What do you mean by “exactly why”.

Are you referring to the fact that I introduced a slightly buggy feature and finally fixed it in this PR?

Or is it the fact that treesitter and the latest version of neovim are malfunctioning?

tonyjara commented 11 months ago

This is exactly why I stopped using this. Thank you!

What do you mean by “exactly why”.

Are you referring to the fact that I introduced a slightly buggy feature and finally fixed it in this PR?

Or is it the fact that treesitter and the latest version of neovim are malfunctioning?

Malfunctioning 😁

gungun974 commented 11 months ago

So for information, I still solved a problem from my modifications from the last time we added.

When you close the regex like this example bellow:

const myRegex = /theRegex/

You had an error because the code is trying to observe the type of an element that does not exist (and will never exist).

With this last commit 9e7256f7e8c22273c9d677c179eca7446d03d0e4 that I just push. I avoid making this mistake.

rmzg commented 10 months ago

@gungun974 Hey, sorry to bother you, I appreciate your work updating this plugin, but I'm having a weird issue where renaming some tags works (ciwspan to change <h1></h1> to <span></span> but it doesn't work for <div></div> tags. I feel like it's something obvious I'm missing, but I can't figure it out, do you happen to know?

Update: this appears to be specific to vue files and only certain vue files, for example, this file:

<script></script>
<template>
  <div>
    <div>
      <div></div>
    </div>
  </div>
</template>

If I open this with neovim, nothing I can do will get the div tags to rename both open and closing tags.

stale[bot] commented 8 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.