JoosepAlviste/nvim-ts-context-commentstring

Top-level jsx elements cannot be uncommented

desi-belokonska opened this issue ยท 8 comments

comment-issue.mov

Problem

Commenting inner jsx elements works fine, but when doing it on the top-level element it only works the first time - every subsequent toggling just uses the default // comment.

Possible source

Did some digging - the function

function M.get_node_at_cursor_start_of_line(only_languages, winnr)

returns a <node jsx_opening_element> the first time and it correctly comments it out with {/* %s */}. When trying to uncomment, however, it returns a <node object>, possibly because it recognizes the { as the start of an object.
Hope this helps!

Hey @desi-belokonska! I've also run into the same issue (and there's an issue for this already as well: #11).

You're correct in the source of the issue. But I'm wondering what the correct behaviour in this case would even be to be honest. It feels like // would be the more correct commentstring because this is valid JSX:

(
  // <div>
    <h1>Hello</h1>
  // </div>
)

But this is invalid:

(
  {/* <div> */}
    <h1>Hello</h1>
  {/* </div> */}
)

So, it seems like the perfect solution would be to check if jsx_element is wrapped by parenthesized_expression. In that case, we should use the // commentstring instead.

I had an idea for a way to configure parent element checks similarly to this:

  tsx = {
    jsx_element = {
      __default = '{/* %s */}',
      __parent = {
        parenthesized_expression = '// %s',
      },
    }
  },

This would also probably fix #22. However, the parent queries would even be worth the effort to implement.

Anyways, I don't think I'll implement this in the near future, but it would be a nice addition and I would gladly accept a contribution ๐Ÿ˜ƒ

Hello, I've facing the same issue in JSX code and there seems to be no activity regarding this. Just want to make sure whether there is a solution to this issue and that I am not missing something.

Hey @vishalbalaji! This would be a pretty big change and I haven't really had a chance to look into it. Since it's such a small edge case, then I'm not even sure if there is any point in spending too much time on it. I would gladly accept a PR though ๐Ÿ˜„

Personally, I don't think being able to comment out the first line of JSX is all that useful (don't think I've ever wanted to do this), but maybe I'm missing something?

Hey @JoosepAlviste, I have mainly noticed this being an issue when commenting out entire components. All the code even within the component becomes uncommentable as seen in the GIF below. I'm not entirely sure, but I believe this is because the parent tag is then identified as ERROR by treesitter as the commented top-level tag is not valid JSX.

Peek 2022-08-30 20-16

2022-08-30_20:20:22_scrot

Hey @JoosepAlviste, I have mainly noticed this being an issue when commenting out entire components. All the code even within the component becomes uncommentable as seen in the GIF below. I'm not entirely sure, but I believe this is because the parent tag is then identified as ERROR by treesitter as the commented top-level tag is not valid JSX.

Are you saying that its a bug in treesitter?

I don't think it's really a problem in Treesitter or any parsers, because this is not valid jsx syntax and probably should result in a broken syntax tree:

(
  {/* <div> */}
   <h1>Hello</h1>
  </div>
)

Ideally, this plugin should probably be smart enough to be able to comment from the first line of JSX like so:

(
  // <div>
    <h1>Hello</h1>
  </div>
)

Which should mean that the first line can be un-commented based on the standard rules we already have.

Currently, there's no easy way to configure this plugin to handle the above case, but something like what I mentioned above (#29 (comment)) should make this possible. Implementing that would probably take quite a bit of effort though.

It seems like this is a rather important issue for folks though, so we could hard-code a workaround in this plugin. Something like "if language is jsx and we're on the first line of a JSX statement, use a standard JS comment". It shouldn't be too tricky to get this to work, probably just requires some fiddling.

I don't know how much time I would have to work on this though, so I would gladly accept a PR with a similar hacky solution ๐Ÿ˜„

I'd try, but I barely know lua. Is there a good place to learn neovim plugin development? @JoosepAlviste

Hi, I have the same problem and I want to help in some way. I made a simple keymap to solve this specific cases.

vim.keymap.set("v", "<leader>gc", ":s/{\\/\\* // | '<,'>s/ \\*\\/}//<cr> | :noh<cr>", { silent = true })

Maybe, because this is a tricky case, we can add some extra keymaps or some config that let config a disabled keymap to execute some similar code as the keymap I put above, I don't know.

video.mp4