AndrewRadev/splitjoin.vim

JSX split issue

LandonSchropp opened this issue · 6 comments

Hello again!

I recently ran across a bug in my JSX code. He's the offending line:

<Tag category={ ARTICLE_CATEGORY } onClick={ () => toggleCategory(ARTICLE_CATEGORY) } />

if I hover my cursor over the beginning of category and run gS, I end up with the following:

<Tag
category={ ARTICLE_CATEGORY }
onClick={ () =
> toggleCategory(ARTICLE_CATEGORY) } />

Instead, I should get this:

<Tag
category={ ARTICLE_CATEGORY }
onClick={ () => toggleCategory(ARTICLE_CATEGORY) } 
/>

It seems like Splitjoin is confusing the > in the inline arrow function with the closing angled bracket.

Thanks!

I can't seem to replicate the issue locally, although I have a JSX plugin that might be interfering. I also tried writing a test which should run with no plugins, and that one passed... but I'm compiling git from the latest HEAD, so it might be a matter of changes to the runtime files.

Could you report what the filetype in this example is? It should be javascriptreact, but it's the first thing to check. It would also be useful if you could report what syntax group is rendered on that > by putting the cursor on it and executing :echo synIDattr(synID(line("."), col("."), 1), "name"). It's also important to check whether you have some JSX plugin installed in your own config.

As a side note, I'm guessing that JSX is somewhere inside a javascript expression, so it highlights properly, right? Something like:

return (
  <Tag category={ ARTICLE_CATEGORY } onClick={ () => toggleCategory(ARTICLE_CATEGORY) } />
);

Thanks for the quick response Andrew!

I can confirm that the file type on the page is javascriptreact.

If this is something to do with syntax, it may be relevant that I'm using LunarVim, which uses NeoVim's new Treesitter parser for syntax highlighting. So when I run the command you mentioned, the echo statement prints out a blank line. The line does highlight properly, but if SplitJoin is dependent on syntax groups then maybe that's the core issue.

If more context is helpful, the file is public and can be viewed here.

If this is something to do with syntax, it may be relevant that I'm using LunarVim, which uses NeoVim's new Treesitter parser for syntax highlighting.

The way that I use syntax highlighting is for stuff like skipping regex matches inside strings. Imagine <div data-something="2 > 1"> -- If I'm looking for a closing > that maches the opening <, I'll hit the one inside the string first. So, what I can do is ask Vim "what is the syntax expression at the match position", and if it's a string or comment, keep going to the next one. There's some good examples in the documentation of :help searchpair().

For good or ill, this turns out not to be the issue here -- the bug comes from va< not knowing to ignore stuff inside of react's { brackets. It skips over strings, but not over javascript expressions. It was working for me by accident, since I didn't have html_attributes_bracket_on_new_line set.

I could apply a similar workaround and skip syntax that starts with js (though it's a bit more complicated than just that), but I guess that wouldn't fix your problem in particular. I've added a regex check for => and pushed it to main, so take a look and let me know if this works for you. This won't fix attr={value > 1 ? ...} for example, but there's not much I can do about figuring out the context without access to syntax groups, so at least lambdas are a fixable common case.

That did the trick! I understand what you mean about this being a tricky problem to solve.

Just out of curiosity, have you ever considered doing a version of SplitJoin that uses Neovim's Treeshaker parser? I know you're currently maintaining maximum compatibility, but an AST seems like it's the long-term solution to some of the tricky edge cases you mentioned, and could make it easier to support other languages in the future.

Thanks for taking care of this! I appreciate it. 🙂

Just out of curiosity, have you ever considered doing a version of SplitJoin that uses Neovim's Treeshaker parser?

Doing this would enable some particular transformations to work that couldn't before, but it would be just the same amount of work, if not more. Even with the AST, you still need to work with the original code, so there's still a bunch of logic required. With Vim, I can at least apply all the standard motions and operate on the text piece-by-piece. I don't have a mental model of how this would even work with a treesitter-based solution.

This particular problem is solveable by having the syntax of the match at the cursor position -- "Am I in javascript right now?". I don't see a technical reason why Neovim's treesitter integration wouldn't define the exact same syntax groups, or at least stub the synID family of functions to return the right thing. But it doesn't, which completely disables some perfectly reasonable ways of saying "find a match, skip over strings or comments". Part of the problem is maybe the current level of (in)stability -- it all seems way too hacky right now to use in any reliable way. Last I checked, the markdown parser was crashing Neovim, which is really not something I'd like to see from a syntax highlighting tool. There's theoretical promise in treesitter, but I'm happy to wait until it's stable and integrated and people use it productively before putting in the time and energy myself. If it ever happens :).

Fair enough. Thanks for the explanation!