frostplexx / mason-bridge.nvim

Automatically register linters and formatters installed in mason.nvim
MIT License
32 stars 2 forks source link

Registration failed for ruff and shfmt #5

Closed trkwyk closed 4 months ago

trkwyk commented 4 months ago

Thank you for the fantastic plugin! I've always wanted to find some plugins that do auto registration after null-ls having been archived and finally I found this. I want to report some bug here. First is, some formatters are also linters like ruff so https://github.com/frostplexx/mason-bridge.nvim/blob/3fe9863bc99a9ddd8e99e8c5e217dd89ad062164/lua/util/bridge.lua#L34-L39 the else if logic is not necessarily true. Second is, I'm not sure using the language section in mason packages to register is the right way to go. For example, shfmt's language section shows Bash, Mksh and Shell whereas the file type for shell scripts (bash, sh, ash etc) is sh (zsh, csh are some special ones) so the formatters/linters don't register correctly. Sure for most of the languages this seems to be okay and I see you have a mapping module for exceptions but I don't really think that's the best practice.

frostplexx commented 4 months ago

Thank you for your feedback. As for your first point I fixed that issue and published a new release. Regarding your second point I know that just guessing that the filetype and language isn't the most ideal approach but i couldn't think of a better one. mason-nvim-dap which I used as a reference while writing this plugin is also maintaining a mappings file: https://github.com/jay-babu/mason-nvim-dap.nvim/blob/3614a39aae98ccd34124b072939d6283853b3dd2/lua/mason-nvim-dap/mappings/filetypes.lua#L4C1-L19C2 I'm also giving the users the ability to manually override/add definitions as mentioned in the readme https://github.com/frostplexx/mason-bridge.nvim/blob/3fe9863bc99a9ddd8e99e8c5e217dd89ad062164/README.md?plain=1#L77C1-L87C3 If someone can think of a better approach for handling filetypes I'm happy to implement that.

trkwyk commented 4 months ago

Yes the filetype is a bit tricky. This just comes to my mind, maybe you could somehow utilise the vim.filetype.match api? https://github.com/neovim/neovim/blob/5c33815448e11b514678f39cecc74e68131d4628/runtime/doc/lua.txt#L2750 First you find some language to common extension mapping source, then map the mason languages to extensions, and then run vim.filetype.match({ filename="foo.extension" }) where foo is just some pseudo name (to fool around the filename and pattern check) and extension is the respective extension obtained last step. Well the api has mechanisms other than just extension to decide the file type (like inspecting the buffer content which we don't have but I'm not sure how it behaves if we supply the filename alone, it's certainly legit though) so ideally the extension obtained last step should be distinct enough or maybe you could tweak the table https://github.com/neovim/neovim/blob/5c33815448e11b514678f39cecc74e68131d4628/runtime/lua/vim/filetype.lua#L176 a bit and hard code that into the plugin to ensure unique results. Anyway, this is just some of my ideas. Feel free to ignore them if that's too cumbersome it's certainly neither simple nor efficient. I imagine a more complete mapping list for the outliers is better if not always fail-safe. And even better let mason.nvim specify the filetype for every package just like lspconfig there would be no such hustle at all... The plugin as is is good enough! Feel free to close this issue. Thanks for the fix!

frostplexx commented 4 months ago

I don't think using vim.filetype.match is the way to go as we would have to still maintain a mapping but additionally have one extra translation step: Language -> Extension (manual mapping) -> Filetype. With the current approach the map simply directly translates Language -> Filetype, which is a bit simpler as it 1. Saves one computation step and 2. There can be many more extensions for one language than there are languages that are the same filetype at least in my experience. Looking around a bit more what other plugins are doing they all maintain hardcoded mappings: https://github.com/williamboman/mason-lspconfig.nvim/blob/a4caa0d083aab56f6cd5acf2d42331b74614a585/lua/mason-lspconfig/mappings/server.lua#L6C1-L216C2

I'm going to keep language handling as is for now and maybe switch it to a pure mappings based approach in the future if i discover that just using the language as the filetype doesn't work for too many languages.

trkwyk commented 4 months ago

Actually the mason-lspconfig mapping is not related to file type but rather as it said "Maps lspconfig server config name to its corresponding package name". However, because all the lsp servers ultimately have to be registered through lspconfig itself, and lspconfig has every server file type specified for example https://github.com/neovim/nvim-lspconfig/blob/b124ef3bd4435a6db7ff03ea2f5a23e1e0487552/lua/lspconfig/server_configurations/lua_ls.lua#L16 so ideally if mason.nvim could do this too all of mason-related repo maintainers could have an easier life.

frostplexx commented 4 months ago

Seems like i sent the wrong example. mason-lspconfig.nvim also has a map of filetype to language server https://github.com/williamboman/mason-lspconfig.nvim/blob/a4caa0d083aab56f6cd5acf2d42331b74614a585/lua/mason-lspconfig/mappings/filetype.lua#L3C1-L243C2 This is still not the same thing as I'm doing but its the same idea. As to why mason.nvim doesnt do this i think its mostly because its outside of masons scope. They are a package manage which only downloads, updates and uninstalls its tools. The language parameter is not used anywhere except for search from what I've seen because nvim only works using filetypes. But I agree that it would be much more convenient for plugin devs if they included filetypes too.

frostplexx commented 4 months ago

In this comment https://github.com/williamboman/mason.nvim/pull/593#discussion_r1002737803 the maintainer of mason.nvim mentions that language entries are only for presentational purposes.