search

JoosepAlviste / nvim-ts-context-commentstring

Neovim treesitter plugin for setting the commentstring based on the cursor location in a file.
MIT License
1.18k stars 35 forks source link

feat: relay on languagetree commentstring as much as possible #86

Open mortezadadgar opened 1 year ago

mortezadadgar commented 1 year ago

~~The idea is taken from mini.comment but it's a little simpler as i don't why languagetree children need to be traversed as well: https://github.com/echasnovski/mini.comment/blob/main/lua/mini/comment.lua#L359~~

after this change merged i think many languages config can be removed as they're obtained from languagetree there are exceptions like html and jsx that need to stayed for now.

mortezadadgar commented 1 year ago

moved the logic to a separate function and removed preconfigured commentstrings not sure rescript, twing and graphql need to be removed as well the code is mostly is taken from mini.comment i will see if i can merge it into check_node() but it seems to be so complicated

JoosepAlviste commented 1 year ago

This looks like a great idea! I'd bring out a couple things for now (as this PR is still a draft):

  1. The filetype commentstring checking logic probably doesn't need to be inside check_node as there we're looping through all the nodes inside a language tree. The filetype commentstring detection is relevant for language trees rather than nodes. I'm not sure what the best place to put the logic would be, though.
  2. One problem with the current location is that if the cursor is in a css block inside tsx, for example, then get_node_at_cursor_start_of_line would return the tsx language tree, rather than the css one, which would get the commentstring from tsx and wouldn't trigger your logic. A visual example:
const MyStyledComponent = styled`
  |color: red;
  ^ cursor is here
`

where get_node_at_cursor_start_of_line would check the current node (css), see that it's not in the languages config, check it's parent (tsx), see that it IS in the config, and use that for the next calculations. Hopefully that made sense 😅

  1. Removing the configs for languages with multiline comments (e.g., lua) would mean that the advanced commenting integration with Comment.nvim wouldn't be able to do multiline comments. I would still keep those in the configuration.
mortezadadgar commented 1 year ago

thanks for suggestions I will take a look but i think we should take away removing commentstring from config for now it seems to be tricky

echasnovski commented 1 year ago

The idea is taken from mini.comment but it's a little simpler as i don't why languagetree children need to be traversed as well: https://github.com/echasnovski/mini.comment/blob/main/lua/mini/comment.lua#L359

The way it works in 'mini.comment' is by traversing all possible trees in the buffer but checking only those that contain reference position. It also makes sure that the "most specific" child provides the 'commentstring' (hence the check for strictly increased level of nesting). I think that in theory there can be complex situations in which a proper source for 'commentstring' is chosen. For example, if there is a language tree A (providing its own 'commentstring') with children AA and AB (also with own values), and AA has also a children AAA (with fifth value). Which value of 'commentstring' should be chosen? 'mini.comment' goes for the language tree with highest nesting (first among those ones, to be precise).

mortezadadgar commented 1 year ago

@JoosepAlviste this pr is almost done i just don't which languages can be removed from the config without breaking anything one idea is to leave it as it's but that would make this pr useless