WhoIsSethDaniel / mason-tool-installer.nvim

Install and upgrade third party tools automatically
MIT License
444 stars 16 forks source link

Remove mason-lspconfig #42

Closed smartinellimarco closed 4 months ago

smartinellimarco commented 7 months ago

Hello! First of all, great plugin!

I noticed an issue that arises from this plugin invoking require('mason-lspconfig') as soon as require('mason-tool-installer') is called, that could disrupt the flow of certain Neovim configurations.

Consider the following sequence (simplified):

Subsequently, mason-lspconfig triggers this check

The premature require('mason-lspconfig') could result in an error, as mason takes a brief moment to indicate readiness by setting the 'has_setup' flag to True.

This depends on how the user plugins are specified, but it seems prudent to just use the mason package names for 'ensure_installed' and remove the dependency.

Thanks in advance!

WhoIsSethDaniel commented 7 months ago

It's not a dependency. If you don't have mason-lspconfig this plugin continues to work. However I do see the point about loading mason-lspconfig too soon being a problem. I can look at that, but I don't think mason-lspconfig will be removed as optional. The ability to use either mason names or lspconfig names was a common request until that code was added.

I don't use lazy myself so I don't know if this can be remedied using it in a different way. I quick fix would be to allow the user to explicitly state "I don't want mason-lspconfig to be loaded by this module". But I need to look at the code...which I will attempt to do this weekend.

WhoIsSethDaniel commented 7 months ago

that could disrupt the flow of certain Neovim configurations

Can you describe your particular situation in more detail? I don't know for certain but it may help.

mehalter commented 4 months ago

I think it would be nice to not remove this mapping feature but maybe make it able to be opted out of. That way the user can decide if they want to not use the mapped names from those plugins and instead opt for heavier lazy loading