-
Notifications
You must be signed in to change notification settings - Fork 787
feat: add diagnostic support for vhdl with vcom and ghdl #1207
base: main
Are you sure you want to change the base?
Conversation
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.
This looks good overall. I left a couple of comments, and I also wanted to mention that it looks like there is a ghdl language server
and it's supported by nvim-lspconfig. I know nothing about this ecosystem, but if that works, it may be an easier solution, since actual LSP servers will provide a better experience than null-ls.
to_stdin = false, | ||
to_temp_file = true, | ||
from_stderr = true, | ||
args = { "-a", "--std=08", "--workdir=/tmp", "$FILENAME" }, |
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.
I know nothing about this language or CLI tool, but judging from the documentation, it looks like there are multiple standards. Is this something we want to always set, or should we leave it up to users to configure?
to_stdin = false, | ||
to_temp_file = true, | ||
from_stderr = true, | ||
args = { "-a", "--std=08", "--workdir=/tmp", "$FILENAME" }, |
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.
Also, just so I understand: is the --workdir
flag necessary because the program will otherwise compile and leave files in the current directory? If this is necessary then we may want to mention that the source will not work as expected on platforms that don't use /tmp
.
filetypes = { "vhdl" }, | ||
generator_opts = { | ||
command = "vcom", | ||
args = { "-2008", "-quiet", "-lint", "-work", "/tmp/vcom_work", "$FILENAME" }, |
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.
Same comments for above apply here.
@jose-elias-alvarez I use mason.nvim, which is supposed to be the successor for nvim-lspconfig. I didn't see any reference to ghdl. |
I don't use mason.nvim, but I don't think it's supposed to replace nvim-lspconfig. nvim-lspconfig's role is to configure language servers, while (as I understand it) Mason's role is to manage the installation of language servers and other tools. Before moving forward with this PR, it would be good to confirm whether the language server provides the same functionality. |
Sorry for that. I think I confused nvim-lspconfig with nvim lsp installer. I don't use nvim-lspconfig directly, so I wouldn't know if the language server provides the same functionality. I user mason to do all the configuration / downloading of the lsp servers. |
That's fine then, let me know what you think about the other comments I left. There's also a failing CI step from |
No description provided.