set up keymaps when plugin is lazy loaded

[matthewsia98]

Addresses #72

Alternatively, we can skip the checks on setup and just expose the set_default_keymaps functions and leave it to the user to call them if they are lazy loading the plugin. Something like

return {
    "kiyoon/jupynium.nvim",
    build = "pip3 install --user .",
    event = "BufWinEnter *.ju.py",
    config = function()
        local jupynium = require("jupynium")
        jupynium.setup({})
        jupynium.set_default_keymaps()
        require("jupynium.textobjects").set_default_keymaps()
    end
}

[kiyoon]

You can instead use list_wildcard_match and then remove the for loop?

[kiyoon]

Can you also add a comment that this part is needed when you add lazy loading like event = "BufWinEnter *.ju.*" in lazy.nvim?

[kiyoon]

If you lazy load it this way, what happens to the vim commands? Does :JupyniumStartAndAttachToServer work, or with the same reason it won't be initialised?

Also, will auto attach to server work?

[kiyoon]

If you lazy load it this way, what happens to the vim commands? Does :JupyniumStartAndAttachToServer work, or with the same reason it won't be initialised?

Also, will auto attach to server work?

I think the commands are loaded alright, but the auto starting server needs update as well. We can address this in a separate PR.

[thesimonho]

this is a broader but related issue:

is this the best way to do buffer-local keybinds? because the current implementation, and in this PR, non-default/user-defined keymaps remain global, requiring the user to set up autocommands themselves

would it not make more sense to expose an on_attach and on_detach function in the config, which gets called by jupynium at the relevant time? that way the user can choose to pass their own keymap function (or the default keymap function) into on_attach and both just get handled the same way instead of needing different/extra function calls?

[kiyoon]

Thanks for the idea, @sho-87!

I get your point, but for things as simple as keymapping, I prefer not to add too many configurations that will make it harder to understand the structure, maintain the plugin and deprecate things at some point.
For example, lazy.nvim plugin manager also allows you to add keybindings directly in there. Some people may prefer using that way instead.

I added the default keybindings not because I think that'd fit for everyone, but more because it'd make it easier to get started with the plugin and understand how things work. I still think that the best way to use the plugin is to turn off the default keybindings and add them on your own.

[kiyoon]

@sho-87 Also, the current autocmd solution is not the best solution because it will remap the keybindings every time you enter *.ju.* files. It won't cause issues for the most of the time, but it can conflict with some plugins that are set later on. So unless we really improve the logic, there's no need to implement the hook because it's as simple as one autocmd anyway.

[thesimonho]

I think this merge broke custom keymaps on my end - can you check if this is also the case on yours? @kiyoon @matthewsia98

after connecting to a server, the default keymaps always show up even though my config has them turned off

reverting to the previous commit (93e4690) makes things work as expected

[kiyoon]

Interesting, maybe the setup gets automatically called with the default configuration due to plugin/jupynium.vim.

I'll probably need to move this into after/

[matthewsia98]

Hmmmm interesting indeed.
I added some print statements in the setup call like

function M.setup(opts)
    print("setup")
    print(vim.inspect(opts))
end

On opening a *.ju.* file it prints setup twice.
First with an empty table.
Second with my actual config.

[kiyoon]

Does the fix work if you pull the recent version?

[matthewsia98]

No..setup still gets called twice, first with {} and second with the actual user config

[thesimonho]

yeah latest version still shows default binds

[kiyoon]

Weird, it only gets called once for me

[kiyoon]

Okay, I see that it gets called twice if the plugin is lazy-loaded. So even the after/ will be called first. Maybe I can remove the auto-calling setup completely, or figure out a different way to set default keymaps when lazy loading. (like mentioned on the top of this PR)

What do you think?

[matthewsia98]

Can we not just add a !has("nvim") to the setup call in after/

[kiyoon]

Can we not just add a !has("nvim") to the setup call in after/

This plugin is only compatible with neovim anyway. This is just there to make it possible to use without calling the setup function. Other plugins have this too. I thought lazy.nvim should call configuration before loading this file.

[kiyoon]

I'll just remove the keymapping part for now. If you're lazy loading, you'll need to deal with the consequences. So you can do the following as you said:

return {
    "kiyoon/jupynium.nvim",
    build = "pip3 install --user .",
    event = "BufWinEnter *.ju.py",
    config = function()
        local jupynium = require("jupynium")
        jupynium.setup({})
        jupynium.set_default_keymaps()
        require("jupynium.textobjects").set_default_keymaps()
    end
}

[kiyoon]

I just made another fix. @sho-87

[thesimonho]

working now, thanks!