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

fix: support compound file-types #3145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+22 −13
Open

fix: support compound file-types #3145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion lua/lspconfig/server_configurations/elmls.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ return {
filetypes = { 'elm' },
root_dir = function(fname)
local filetype = api.nvim_buf_get_option(0, 'filetype')
if filetype == 'elm' or (filetype == 'json' and fname:match 'elm%.json$') then
if util.ft_matches(filetype, 'elm') or (util.ft_matches(filetype, 'json') and fname:match 'elm%.json$') then
return elm_root_pattern(fname)
end
end,
Expand Down
33 changes: 21 additions & 12 deletions lua/lspconfig/util.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,21 @@ M.default_config = {
-- global on_setup hook
M.on_setup = nil

---@param filetype string the filetype to check (can be a compound, dot-separated filetype; see |'filetype'|)
Copy link
Member

Choose a reason for hiding this comment

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

this new function has no docstring explaining its purpose. and this PR has a wall of text discussing it.

how in the world is anyone supposed know when to use this

Copy link
Author

Choose a reason for hiding this comment

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

apologies, I didn't realize that it was unclear. I've added documentation. let me know if it's unclear

---@param expected string|string[] the filetype(s) to match against
---@return boolean
function M.ft_matches(filetype, expected)
Copy link
Member

Choose a reason for hiding this comment

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

there has vim.fs module.

Copy link
Author

Choose a reason for hiding this comment

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

sorry, can you explain? I don't see anything in vim.fs that does this. vim.fs seems to deal mostly with filesystem operations. I'm not actually reading files here, I am only splitting and comparing filetype strings

Copy link
Member

Choose a reason for hiding this comment

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

okay see vim.filetype moudle. vim.filetype.match will return the filetype if it already in default detect table or register by vim.filetype.add . we don't want add more utils function now

Copy link
Author

Choose a reason for hiding this comment

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

vim.filetype.match takes an existing buffer and returns the filetype (using the rules defined in filetype.lua). the result of that might be (for example) c.doxygen. see |'filetype'|:

						*'filetype'* *'ft'*
'filetype' 'ft'		string	(default "")
			local to buffer  |local-noglobal|
...
	Setting this option to a different value is most useful in a modeline,
	for a file for which the file type is not automatically recognized.
	Example, for in an IDL file: >c
		/* vim: set filetype=idl : */
<	|FileType| |filetypes|
	When a dot appears in the value then this separates two filetype
	names.  Example: >c
		/* vim: set filetype=c.doxygen : */
<	This will use the "c" filetype first, then the "doxygen" filetype.
	This works both for filetype plugins and for syntax files.  More than
	one dot may appear.
	This option is not copied to another buffer, independent of the 's' or
	'S' flag in 'cpoptions'.
	Only normal file name characters can be used, `/\*?[|<>` are illegal.

In other words:

local ft = vim.filetype.match({buf=0})
ft == "c.doxygen"
util.ft_matches(ft, 'idl') == false
util.ft_matches(ft, 'c') == true
util.ft_matches(ft, 'doxygen') == true

Copy link
Member

@glepnir glepnir May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

you can also pass filename into match

        -- Using a filename without a buffer
        vim.filetype.match({ filename = 'main.lua' })

        -- Using file contents
        vim.filetype.match({ contents = {'#!/usr/bin/env bash'} })

Copy link
Author

@baodrate baodrate May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yes, vim.filetype.match can take a filename and give us a filetype. but what we want is to take that filetype and check it against a list of filetypes (e.g. the ones supported by a particular lsp server).

i.e.

local ft = vim.filetype.match({ filename = 'main.lua' })
ft == "lua"
local filetypes_supported_by_current_client = { "lua", "sh", "c" }
util.ft_matches(ft, filetypes_supported_by_current_client) == true

Usually this would just be a == comparison but because a filetype can contain multiple filetypes (e.g. c.doxygen), we have to split it first.

local ft = vim.bo.filetype
ft == "yaml.ansible" -- this is not a filename! it is a _compound_ filetype
util.ft_matches(ft, "ansible") == true
util.ft_matches(ft, "yaml") == true

notice that vim.filetype.match doesn't help us do this comparison! it takes a buffer/file/filepath and gives us a filetype. but we don't need that. we have a filetype, it is provided by the caller. We are trying to match that against a list of acceptable filetypes, also provided by the caller.

Copy link
Member

@glepnir glepnir May 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

thanks for your kind reply . does register a filetype by using vim.filetype.add for a special file not make sense in user side ?

Copy link
Author

Choose a reason for hiding this comment

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

A completely unique type is not desirable because we want to use the existing behaviors defined for the primary filetype. see mfussenegger/nvim-ansible or pearofducks/ansible-vim. They both set ft=yaml.ansible.

You want ansible scripts to have the yaml filetype because that's what they are; we want (neo)vim to keep recognizing them as yaml files. if we set ft=ansible only, we lose the syntax highlighting/match_words/etc defined for yaml files.

But the plugin additionally sets various useful options like keywordprg/path/iskeyword/isfname for ansible file types. Hence the reason for multiple file types.

The problem is that right now, nvim-lspconfig diverges from standard vim behavior because filetype matching operates on the entire vim.bo.filetype as a whole, rather than splitting it into individual file types. And every server has to be configured for every possible combination of multiple filetypes. see:

nvim-lspconfig/lua/lspconfig/server_configurations/yamlls.lua

Lines 4 to 6 in a3d9395

default_config = {
cmd = { 'yaml-language-server', '--stdio' },
filetypes = { 'yaml', 'yaml.docker-compose', 'yaml.gitlab' },

(notice that yaml.ansible is not included.

With this change, you can instead just specify yaml, and we'll match as long as one of your filetypes matches yaml

expected = type(expected) == 'table' and expected or { expected }
for ft in filetype:gmatch '([^.]+)' do
for _, expected_ft in ipairs(expected) do
if ft == expected_ft then
return true
end
end
end
return false
end

function M.bufname_valid(bufname)
if bufname:match '^/' or bufname:match '^[a-zA-Z]:' or bufname:match '^zipfile://' or bufname:match '^tarfile:' then
return true
Expand Down Expand Up @@ -346,10 +361,8 @@ function M.get_active_clients_list_by_ft(filetype)
local clients_list = {}
for _, client in pairs(clients) do
local filetypes = client.config.filetypes or {}
for _, ft in pairs(filetypes) do
if ft == filetype then
table.insert(clients_list, client.name)
end
if M.ft_matches(filetype, filetypes) then
table.insert(clients_list, client.name)
end
end
return clients_list
Expand All @@ -362,10 +375,8 @@ function M.get_other_matching_providers(filetype)
for _, config in pairs(configs) do
if not vim.tbl_contains(active_clients_list, config.name) then
local filetypes = config.filetypes or {}
for _, ft in pairs(filetypes) do
if ft == filetype then
table.insert(other_matching_configs, config)
end
if M.ft_matches(filetype, filetypes) then
table.insert(other_matching_configs, config)
end
end
end
Expand All @@ -377,10 +388,8 @@ function M.get_config_by_ft(filetype)
local matching_configs = {}
for _, config in pairs(configs) do
local filetypes = config.filetypes or {}
for _, ft in pairs(filetypes) do
if ft == filetype then
table.insert(matching_configs, config)
end
if M.ft_matches(filetype, filetypes) then
table.insert(matching_configs, config)
end
end
return matching_configs
Expand Down
Loading