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.

Sign up for GitHub

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

Jump to bottom

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5bece0f
feat: relay on languagetree commentstring as much as possible
mortezadadgar Nov 20, 2023
0d34afd
remove some preconfigured commentstring
mortezadadgar Nov 20, 2023
40a0b06
new implementation
mortezadadgar Nov 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 0 additions & 18 deletions lua/ts_context_commentstring/config.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,29 +59,11 @@ M.config = {

languages = {
-- Languages that have a single comment style
astro = '<!-- %s -->',
c = { __default = '// %s', __multiline = '/* %s */' },
css = '/* %s */',
glimmer = '{{! %s }}',
graphql = '# %s',
handlebars = '{{! %s }}',
html = '<!-- %s -->',
lua = { __default = '-- %s', __multiline = '--[[ %s ]]' },
nix = { __default = '# %s', __multiline = '/* %s */' },
php = { __default = '// %s', __multiline = '/* %s */' },
python = { __default = '# %s', __multiline = '""" %s """' },
rescript = { __default = '// %s', __multiline = '/* %s */' },
scss = { __default = '// %s', __multiline = '/* %s */' },
sh = '# %s',
bash = '# %s',
solidity = { __default = '// %s', __multiline = '/* %s */' },
sql = '-- %s',
svelte = '<!-- %s -->',
twig = '{# %s #}',
typescript = { __default = '// %s', __multiline = '/* %s */' },
vim = '" %s',
vue = '<!-- %s -->',
zsh = '# %s',
Comment on lines -62 to -84
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the language configs that:

  • Do not have a default in Neovim core
  • Have an extra multiline comment style config

as removing those would result in breaking changes and would require people to change their configs after updating this plugin. I would very much like to avoid any breaking changes, we could group those together with other breaking changes in the future (e.g., removing the config option for good).

Here are the language configs I would keep:

  • astro - no default commentstring
  • c - has a multilline config
  • grimmer - no default
  • graphql - no default
  • handlebars - no default
  • lua - multiline config
  • nix - multiline config
  • php - multiline config
  • python - multiline config
  • rescript - multiline config
  • scss - multiline config
  • solidity - multiline config
  • sql - no default
  • svelte - no default
  • twig - no default
  • typescript - multiline config
  • vue - no default

The others can be deleted, I think.

This at least makes it so that new languages with a single commenting style would work out of the box, and wouldn't need to be configured in this plugin.

We can figure out how to get rid of the multiline comment configs in the future (maybe using :h comments somehow?).


-- Languages that can have multiple types of comments
tsx = {
Expand Down
19 changes: 16 additions & 3 deletions lua/ts_context_commentstring/internal.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,8 @@ end
function M.calculate_commentstring(args)
args = args or {}
local key = args.key or '__default'
local location = args.location or nil
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to keep allowing to pass in a custom location as this is required for the Comment.nvim integration. The commentstring shouldn't always be calculated from the cursor line's start (e.g., when using motions like gciw)


local node, language_tree =
utils.get_node_at_cursor_start_of_line(vim.tbl_keys(config.get_languages_config()), location)
local node, language_tree = utils.get_node_at_cursor_start_of_line()

if not node and not language_tree then
return nil
Expand All @@ -71,9 +69,24 @@ function M.calculate_commentstring(args)
local language = language_tree:lang()
local language_config = config.get_languages_config()[language]

-- Use ts node commentstring if no language is configured
if not language_config then
local cs = M.check_ts_node(language_tree)
if cs then
return cs
end
end

return M.check_node(node, language_config, key)
end

function M.check_ts_node(language_tree)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name for this isn't very indicative of what the function does, I'd rename it to something like "get commentstring from language tree".

Also, it would be nice to have type hints for the parameter and return type.

local cs = vim.filetype.get_option(language_tree:lang(), 'commentstring')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Language of a tree doesn't always equal to the filetype it is usually used. See vim.treesitter.language.get_filetypes() (beware that it is added only in Neovim 0.9.

Copy link
Contributor Author

@mortezadadgar mortezadadgar Nov 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah what i'm doing already is to obtain the language under the cursor and skip the comment type and seems to work fine(missed that part in new implementation)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if this variable is used in a very small scope, I think that it still makes sense to avoid abbreviations in variable names. "cs" makes me think of Counter Strike 😄

Same thing above

if type(cs) == 'string' and cs ~= '' then
return cs
end
end

---Update the `commentstring` setting based on the current location of the
---cursor. If no `commentstring` can be calculated, will revert to the ofiginal
---`commentstring` for the current file.
Expand Down
14 changes: 5 additions & 9 deletions lua/ts_context_commentstring/utils.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,14 @@ end
---Returns the node at the cursor's line and the language tree for that
---injection.
---
---@param only_languages string[] List of languages to filter for, all
--- other languages will be ignored.
---@param location? ts_context_commentstring.Location location Line, column
--- where to start traversing the tree. Defaults to cursor start of line.
--- This usually makes the most sense when commenting the whole line.
---
---@return table|nil node, table|nil language_tree Node and language tree for the
--- location
function M.get_node_at_cursor_start_of_line(only_languages, location)
function M.get_node_at_cursor_start_of_line()
if not M.is_treesitter_active() then
return
end

location = location or M.get_cursor_line_non_whitespace_col_location()
local location = M.get_cursor_line_non_whitespace_col_location()
local range = {
location[1],
location[2],
Expand All @@ -99,7 +93,9 @@ function M.get_node_at_cursor_start_of_line(only_languages, location)
local language_tree = vim.treesitter.get_parser()
-- Get the smallest supported language's tree with nodes inside the given range
language_tree:for_each_tree(function(_, ltree)
if ltree:contains(range) and vim.tbl_contains(only_languages, ltree:lang()) then
-- always exclude the comment language as it's returning invalid
-- commentstring
if ltree:contains(range) and ltree:lang() ~= 'comment' then
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also jsdoc, phpdoc, and probably other parsers as well which don't map directly to a filetype. Do we need to handle those too?

language_tree = ltree
end
end)
Expand Down