-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: vb_ls #3917
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
base: master
Are you sure you want to change the base?
feat: vb_ls #3917
Conversation
Didn't met the repo star requirement though https://github.com/neovim/nvim-lspconfig/blob/master/CONTRIBUTING.md#criteria But glad to see vb.net support outside visual studio, great job! |
local fname = vim.api.nvim_buf_get_name(bufnr) | ||
on_dir(util.root_pattern '*.sln'(fname) or util.root_pattern '*.vbproj'(fname) or util.root_pattern '*.slnx'(fname)) | ||
end, | ||
filetypes = { 'vbnet' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is the "vbnet" filetype coming from, a thirdparty plugin I assume (should mention and link to it in the doc)? The filetype shipped w/ vim is "vb": https://github.com/neovim/neovim/blob/5cf135f9a0ac472621d133bb4a03780a7bff6640/runtime/lua/vim/filetype/detect.lua#L107
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that was one of my questions.
The reason for it is that the built in vb targetting VB6 and vb.net do differ in some areas but I can switch it to vb.
The patched version I have been using is here.
https://github.com/CoolCoderSuper/visualbasic-language-server/blob/main/vim-config/syntax/vbnet.vim
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this config will work for exactly 1 person currently? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use :h vim.filetype.add
to create filetype vbnet
based on the presence of vbproj
, and clarify these setup on docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sharpchen thanks for the guidance. I will look into that and update the docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And for the syntax for vbnet
you should wrap it as a vim plugin so others can install it, should look like https://github.com/vim-scripts/vbnet.vim, not related to ls config but it's worth noting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@justinmk I understand, just to mention the workaround to manage external filetype for language server if needed, it should be added to user config, not related to lspconfig, thanks for clarification :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you both for your feedback.
I created this plugin https://github.com/CoolCoderSuper/vbnet.nvim and added a reference to it in the doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CoolCoderSuper I was expecting https://github.com/CoolCoderSuper/vbnet.nvim to be a pure syntax plugin, it seems you're enforcing user to override vb
to vbnet
, I do think we should leave it to user to decide whether to override or not if distinction between vb6 and vb.net does matter, as you mentioned earlier. something like this:
vim.filetype.add {
extension = {
vb = function(path, _)
if vim.fs.root(path, function(name, _) return name:match('%.vbproj$') end) then
return 'vbnet'
else
return 'vb'
end
end,
},
}
With snippet above, all buffer with extension vb
would be of filetype vbnet
when it find a vbproj
upward, otherwise it would be just vb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sharpchen OK, that makes sense. I have updated it to use that approach.
Yeah, true. Normally would close this until that criteria is met. The docs look good and the config is doing some unusual stuff (which suggests having it will be helpful). Can keep this PR open and see if there are a few more upvotes on it. |
This adds a config for a VB.NET language server.
https://github.com/CoolCoderSuper/visualbasic-language-server
It registers on the vbnet file type where as the default provided by Vim for the .vb extension is vb.
This means the users would need first register .vb and vbnet for it register.
Is that acceptable or should it register on vb?
Is the root_dir setup like this correct?
or should it be simplified like so
This is my first time contributing here so let me know if there is something I'm missing.