feat: warn if executable cannot be resolved from node_modules or fallback

[axieax]

Resolves problem where users may not be aware that a formatter/diagnostic etc. is not running since it cannot be found, which may cause incorrect assumptions that their file has passed formatting/diagnostic checks (e.g. for Prettier, eslint and other modules with dynamic commands).

[jose-elias-alvarez]

I definitely see the issue, but I think this approach may cause performance problems, since when using a global executable, the executable check will run every time the generator runs (which is quite often for diagnostic sources). With this approach, I think we'd need to implement some form of caching, which could be as simple as storing a key-value table of commands that we've already checked (we could even implement this inside of u.is_executable itself).

If you're up for it, a more robust solution might be to determine the ability to execute a command based on the result of vim.loop.spawn, since vim.fn.executable doesn't account for all cases. Neovim switched to this approach for LSP servers in neovim/neovim#16430, and I've been meaning to switch over to it, too (except for healthchecks). This would be a little more complicated, since due to the nature of libuv errors we'd have to catch the error here and bubble it up as a result so we can handle it here.

This would be the most reliable way to check if a command is executable, though, and it would handle your case + all future cases, so I think it might justify a little more work. Again, let me know if you're up for it, and otherwise we can go with a simpler (cached) solution for now.

[axieax]

Ahh I see. I would love to implement the second solution since it'll help out other future cases as well. Tbh, I'm quite busy atm so this may take a while to complete, but I'm happy for someone else to take this as well.

[eshepelyuk]

What about leveraging this https://github.com/rcarriga/nvim-notify ?

I mean, start using vim.notify for reporting executable errors, so users can switch their own notification implementation, if they want.

[jose-elias-alvarez]

What about leveraging this https://github.com/rcarriga/nvim-notify ?

I mean, start using vim.notify for reporting executable errors, so users can switch their own notification implementation, if they want.

This would be a nice improvement (and one we can handle separately from this PR if anyone is interested).

[eshepelyuk]

@jose-elias-alvarez i will try to implement in other PR if I will wail - i will report.

any more suggestions?

[jose-elias-alvarez]

@jose-elias-alvarez i will try to implement in other PR if I will wail - i will report.

any more suggestions?

This logic lives in logger.lua and is currently offloaded to Plenary's log module. The easiest option would be for us to add some additional logic to send messages to vim.notify if the user has enabled it (which I think is a sensible default).

Another option would be to open an upstream PR to enable logging to vim.notify in Plenary. If you're up for it, this would be a nice solution, since it would benefit everybody in the ecosystem, though I'm not 100% sure that they'd accept it, since logs and notifications are not exactly the same thing.

Also, I know other plugins like nvim-lsp-installer have switched to their own implementations. Our use case is simple enough that we could just replace what we have with a custom implementation, though this adds some maintenance burden / duplication of effort.

[eshepelyuk]

@jose-elias-alvarez i will try to implement in other PR if I will wail - i will report. any more suggestions?

This logic lives in logger.lua and is currently offloaded to Plenary's log module. The easiest option would be for us to add some additional logic to send messages to vim.notify if the user has enabled it (which I think is a sensible default).

Another option would be to open an upstream PR to enable logging to vim.notify in Plenary. If you're up for it, this would be a nice solution, since it would benefit everybody in the ecosystem, though I'm not 100% sure that they'd accept it, since logs and notifications are not exactly the same thing.

Also, I know other plugins like nvim-lsp-installer have switched to their own implementations. Our use case is simple enough that we could just replace what we have with a custom implementation, though this adds some maintenance burden / duplication of effort.

Thnx for detailled explanation.

[eshepelyuk]

@jose-elias-alvarez i believe this should be closed now.

[jose-elias-alvarez]

@jose-elias-alvarez i believe this should be closed now.

If I understand correctly, I think the problem this PR addresses is not quite the same. The issue here is that in certain cases, the executable check doesn't run at all, meaning nothing gets logged and the source fails silently.

[jose-elias-alvarez]

Closing this as #1077 has solved the issue in a more robust way. Thanks for raising the issue!