bergercookie / asm-lsp

Language server for NASM/GAS/GO Assembly
https://crates.io/crates/asm-lsp
BSD 2-Clause "Simplified" License
269 stars 18 forks source link

Have a per directory asm-lsp.toml #153

Closed lu-zero closed 3 weeks ago

lu-zero commented 1 month ago

I'm trying to use asm-lsp on dav1d, one of the problems is that we have assembly for many different architectures so ideally one would want to have a per-directory configuration so the lsp won't try to autocomplete x86 if the directory is arm or riscv.

Ultra-Code commented 1 month ago

I came across a similar issue where I had aarch64 and x86_64 in different folders. Fixing this would improve the user experience in such cases.

WillLillis commented 1 month ago

This is a pretty cool idea. One initial thought I had is to allow the config at the project's root directory to contain an additional field that just lists the project's subdirectories containing other .asm-lsp.toml files. The root config could specify default settings (if any), which the sub configs could then override.

Alternatively the server could do automatic detection by recursively searching from the project's root, but I imagine interacting that much with the fs would greatly impact startup time.

lu-zero commented 1 month ago

Also that way would work fine in dav1d. Right now I'm having troubles convincing the lsp to pick anything not x86 even if I put the toml in the root directory though... and I'm on an arm system. ^^;

Could you please share your vim/neovim configuration so I'm sure I'm not doing something utterly wrong? :)

WillLillis commented 1 month ago

Also that way would work fine in dav1d. Right now I'm having troubles convincing the lsp to pick anything not x86 even if I put the toml in the root directory though... and I'm on an arm system. ^^;

Could you please share your vim/neovim configuration so I'm sure I'm not doing something utterly wrong? :)

My neovim config is here: https://github.com/WillLillis/nvim

Could you share the server's logs when it fails detection in these cases? Typically if it can't find or deserialize a config file it defaults to x86.

lu-zero commented 1 month ago

Another question would be how to get the server logs... :CocOpenLog doesn't show much...

lu-zero commented 1 month ago

using nvim I figured out that [opts] is not optional and it fails to parse the configuration if it is missing. Then looks like treesitter does not grok .macro much and the ISA source may lack some operations. potentially this might be used in the long run.

Ultra-Code commented 1 month ago

One initial thought I had is to allow the config at the project's root directory to contain an additional field that just lists the project's subdirectories containing other .asm-lsp.toml files

Nice ideas 💡 but how about the main .asm-lsp.toml having a sections configuring the subdirectory instead of subdirectories having dedicated .asm-lsp.toml

Eg.

[[project]]
path="to/aarch64"
assembler = ["gas"]
instruction_set = {arm64=true}
opts = { compiler = "zig"}

[[project]]
path="to/riscv"
assembler = ["nasm"]
instruction_set = {riscv=true}
opts = { compiler = "gcc"}

If an option isn't specified in project the default in .asm-lsp.toml is used.

WillLillis commented 1 month ago

One initial thought I had is to allow the config at the project's root directory to contain an additional field that just lists the project's subdirectories containing other .asm-lsp.toml files

Nice ideas 💡 but how about the main .asm-lsp.toml having a sections configuring the subdirectory instead of subdirectories having dedicated .asm-lsp.toml

Eg.


[[project]]

path="to/aarch64"

assembler = ["gas"]

instruction_set = {arm64=true}

opts = { compiler = "zig"}

[[project]]

path="to/riscv"

assembler = ["nasm"]

instruction_set = {riscv=true}

opts = { compiler = "gcc"}

If an option isn't specified in project the default in .asm-lsp.toml is used.

Ah, I like this approach a lot more. Thanks!

WillLillis commented 1 month ago

using nvim I figured out that [opts] is not optional and it fails to parse the configuration if it is missing.

Sorry for missing your reply earlier! The field currently isn't optional, but there isn't a strong reason for this. I can implement this change after (or along with) the other configuration changes outlined in this issue.

Then looks like treesitter does not grok .macro much

Yeah, the tree-sitter grammar we currently use is a best effort and more aimed to provided highlighting for multiple assembly flavors rather than an accurate depiction of the document. I've talked with some of the tree-sitter folks and there are some distant plans to construct a better grammar, but there are a few major blockers to the core lib itself before this is doable (see https://github.com/tree-sitter/tree-sitter/pull/1635 if you're interested). I'm doing my best to get up to speed so that I'll be able to take a stab at some of these issues, but I'm still a ways off. :slightly_smiling_face:

and the ISA source may lack some operations. potentially this might be used in the long run.

That looks very promising, would be happy to migrate over!

lu-zero commented 1 month ago

and the ISA source may lack some operations. potentially this might be used in the long run.

That looks very promising, would be happy to migrate over!

I discovered it just yesterday, and the people that started it are looking for feedback and users, so if you have opinions they would love to hear about it :)

lu-zero commented 1 month ago

Yeah, the tree-sitter grammar we currently use is a best effort and more aimed to provided highlighting for multiple assembly flavors rather than an accurate depiction of the document. I've talked with some of the tree-sitter folks and there are some distant plans to construct a better grammar, but there are a few major blockers to the core lib itself before this is doable (see tree-sitter/tree-sitter#1635 if you're interested). I'm doing my best to get up to speed so that I'll be able to take a stab at some of these issues, but I'm still a ways off. 🙂

I wish I have time to help getting an ungrammar for the rust inline assembly (that would be partially recycled for the gas macro support).