SmiteshP / nvim-navbuddy

A simple popup display that provides breadcrumbs feature using LSP server
Apache License 2.0
762 stars 30 forks source link

Fix: move "Navbuddy" user command to the global scope, so invoking it without LSP attached results in proper error #100

Open sleeptightAnsiC opened 1 month ago

sleeptightAnsiC commented 1 month ago

Hey,

Since user command Navbuddy exists only when LSP is attached, it does not appear in the completion all the time, and invoking it without LSP does not produce any meaningfull error.

The only thing that appears in this case is E492: Not an editor command: Navbuddy which is super confusing, if you don't know/remember how the plugin works.

Creating Navbuddy only when LSP attaches doesn't make much sense, because said cmd does not have any logic on it's own, it calls a wraper and the actual function which already has a branch for dealing with this exact case: https://github.com/SmiteshP/nvim-navbuddy/blob/f22bac988f2dd073601d75ba39ea5636ab6e38cb/lua/nvim-navbuddy/init.lua#L209-L210

So I just turned Navbuddy into global user command, by moving it to M.setup(). This fixes the mentioned issues without introducing any new code.

Although, I wonder why it was implemented like this in the first place? Maybe, I'm missing something here? Some edge case?

Ideally, the best possible behavior would be forcing LSP to start and then, if it won't start, printing the error, but this would be way more risky to implement and harder to maintain.

Cheers!

sleeptightAnsiC commented 1 month ago

Also, an alternative fix is to have additional global cmd which will be shadowed by the local one when LSP attaches. This can be easily implemented when configuring the plugin.

For example, before I created PR I had this inside of my config:

vim.api.nvim_create_user_command("Navbuddy", function ()
    vim.notify(
        "LSP must be attached to the buffer in order to use Navbuddy!",
        vim.log.levels.WARN,
        { title = "nvim-navbuddy" }
    )
end, {})