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

Configure relative priority of built-in comments (ft.lua) and commentstring #109

Closed
smackesey opened this issue Jan 26, 2022 · 7 comments
Closed
Labels
wontfix This will not be worked on

Comments

@smackesey
Copy link

From the README:

Although, Comment.nvim supports neovim's commentstring but unfortunately it has the least priority. The commentstring is taken from the following place in the respective order.

pre_hook - If a string is returned from this method then it will be used for commenting.

ft_table - If the current filetype is found in the table, then the string there will be used.

commentstring - Neovim's native commentstring for the filetype

README then goes on to provide pre_hook code for integrating with JoosephAlviste/nvim-ts-context-commentstring.

Why not let the user decide whether ft_table or commentstring gets priority (ideally configurable per-filetype?). Letting commentstring have priority would eliminate the need for any integration code at all for nvim-ts-context-commentstring.

@smackesey smackesey changed the title Configure relative to priority of built-in comments (ft_table) and Configure relative to priority of built-in comments (ft.lua) and commentstring Jan 26, 2022
@smackesey smackesey changed the title Configure relative to priority of built-in comments (ft.lua) and commentstring Configure relative priority of built-in comments (ft.lua) and commentstring Jan 26, 2022
@numToStr
Copy link
Owner

Why not let the user decide whether ft_table or commentstring gets priority (ideally configurable per-filetype?). Letting commentstring have priority would eliminate the need for any integration code at all for nvim-ts-context-commentstring.

I think you are confused regarding the use of nvim-ts-context-commentstring. How letting the user set priority would remove the need for nvim-ts-context-commentstring integration?

@numToStr
Copy link
Owner

Also, please provide the actual issue that you are facing with the plugin.

@smackesey
Copy link
Author

I think you are confused regarding the use of nvim-ts-context-commentstring. How letting the user set priority would remove the need for nvim-ts-context-commentstring integration?

Because nvim-ts-context-commentstring updates commentstring for you during editing. So, if the user were able to configure the plugin to prioritize commentstring over ft.lua for tsx/jsx, then no pre-hook integration would be needed-- just installing nvim-ts-context-commentstring would be enough. When commenting, Comment.nvim would just read the commentstring already set by nvim-ts-context-commentstring.

To illustrate, this is my current (working) pre-hook configuration. All it does is return commentstring:

pre_hook = function(ctx)
  if vim.bo.filetype == 'typescriptreact' then
    return vim.o.commentstring
  end
end

Also, please provide the actual issue that you are facing with the plugin.

Sorry I was unclear, this is a feature request.

@numToStr
Copy link
Owner

numToStr commented Jan 26, 2022

Because nvim-ts-context-commentstring updates commentstring for you during editing.

Yes and No. The integration that is showcased in the readme doesn't update the commentstring. It just returns that string so that it can be used for comment/uncomment.

So, if the user were able to configure the plugin to prioritize commentstring over ft.lua for tsx/jsx, then no pre-hook integration would be needed-- just installing nvim-ts-context-commentstring would be enough.

You would still need to call something to update commentstring (which I don't recommend), hence pre_hook


I have already thought of this before making this plugin and IMO updating commentstring every time is just not worth. Why not just use that string instead of updating a editor's setting? If you get my point, then your request for "priority" will look redundant :)

@smackesey
Copy link
Author

smackesey commented Jan 26, 2022

We're miscommunicating-- I'm not suggesting that Comment.nvim should update commentstring. I'm pointing out that nvim-ts-context-commentstring already updates commentstring, indepedent of anything Comment.nvim does.

You would still need to call something to update commentstring (which I don't recommend), hence pre_hook

You do not-- that's why the pre_hook I posted (much shorter than the recommended integration) already works. It does not update commentstring, because nvim-ts-context-commentstring does it automatically.

@numToStr
Copy link
Owner

I'm pointing out that nvim-ts-context-commentstring already updates commentstring

This is what I don't recommend because you have an option to use the calculated commentstring which I showed in the readme.

I understand your point but it's the design choice i made with pre_hook and ft_table which I don't intend to change or provide any kind of setting to override that.


IMO the integration code b/w both the plugin is way too verbose. And I believe that can be improved (you can talk to it's author). I am also thinking of making a Comment.nvim only jsx integration as a separate plugin but I am lacking motivation XD

@smackesey
Copy link
Author

OK, thx for engaging.

@numToStr numToStr added the wontfix This will not be worked on label Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants