kylechui / nvim-surround

Add/change/delete surrounding delimiter pairs with ease. Written with :heart: in Lua.
MIT License
3.09k stars 61 forks source link

No longer able to use `buffer_setup` #116

Closed SidOfc closed 2 years ago

SidOfc commented 2 years ago

Hello, first of all, thank you for writing this plugin! I really like it 👍

Checklist

Describe the bug Before the changes in this commit: 306a92830ede75d7d3a48ba04c7e603b73c3f377 I was able to call the buffer_setup method in a FileType autocmd. After this commit I am now receiving an error on config.lua:122.

To Reproduce Step 1: Open Neovim with nvim -u NONE (assuming nvim-surround is installed in packpath) Step 2: Enter :lua require('nvim-surround').buffer_setup() Step 3: You should see the following error:

E5108: Error executing lua .../packer/start/nvim-surround/lua/nvim-surround/config.lua:122: attempt to index field 'keymaps' (a nil value)
stack traceback:
        .../packer/start/nvim-surround/lua/nvim-surround/config.lua:122: in function 'set_keymaps'
        .../packer/start/nvim-surround/lua/nvim-surround/config.lua:268: in function 'buffer_setup'
        ...ck/packer/start/nvim-surround/lua/nvim-surround/init.lua:34: in function 'buffer_setup'
        [string ":lua"]:1: in main chunk

Expected behavior The call to buffer_setup to be successful.

Thank you for your time!

SidOfc commented 2 years ago

I decided to do some additional digging by rolling back 1 commit at a time, and it seems that this error supposedly was always there. I am no longer convinced this is a bug in your plugin at this moment so I'll close it for now. If I find out more I'll add info to this conversation. Apologies for the potential noise!

SidOfc commented 2 years ago

Hmn I'm not sure what it is exactly, the reason I thought it was odd is because I've used it quite often without any warnings and upon updating this issue started to pop up. I've uninstalled all my packer packages, reran the experiment and still same error so I'm a bit clueless how to continue 😅

Instead of digging further I wrote some Lua which fixes the issue for me:

-- what I added
local surround_config = require('nvim-surround.config')
surround_config.user_opts = surround_config.default_opts

-- what I had originally
require('nvim-surround').buffer_setup()

Just putting it out there in case anyone else needs a quick monkeypatch basically :)

kylechui commented 2 years ago

Do you have buffer_setup called without setup already called? (I don't actually know if this is an issue, but I definitely did not account for it).

SidOfc commented 2 years ago

Yep! My goal is to not have any "global" mappings injected by nvim-surround as this interferes with some mappings of my own file explorer plugin (in which having surrounds also wouldn't really make sense).

For this reason I decided to use just the buffer_setup function which did work for a while until I started to see this error.

kylechui commented 2 years ago

@SidOfc I would recommend you just disable everything for global maps, e.g.

require('nvim-surround').setup({
    keymaps = { normal = false, insert = false, --[[ ... ]] },
})

that should disable everything by default, and then buffer_setup should override it to give you buffer-local maps.

SidOfc commented 2 years ago

With this in my packer setup:

    use({
      'kylechui/nvim-surround',
      config = function()
        require('nvim-surround').setup({
          keymaps = {
            insert = false,
            insert_line = false,
            normal = false,
            normal_cur = false,
            normal_line = false,
            normal_cur_line = false,
            visual = false,
            visual_line = false,
            delete = false,
            change = false,
          },
        })
      end,
    })

Calling :lua require('nvim-surround').buffer_setup() results in the error I posted in the issue:

image

This does not seem to look right to me but I could be wrong of course 😅

SidOfc commented 2 years ago

For reference I'm just using :lua require('nvim-surround').buffer_setup() manually from ex-mode after having loaded my vim configuration file (passing an empty table as argument also doesn't make a difference). At this point everything should have been set up if I am understanding correctly.

kylechui commented 2 years ago

Ah this is something that I think is intentional---whatever you call setup with becomes your "global" setup, so wherever you call buffer_setup you'll need to have the table

    keymaps = {
        insert = "<C-g>s",
        insert_line = "<C-g>S",
        normal = "ys",
        normal_cur = "yss",
        normal_line = "yS",
        normal_cur_line = "ySS",
        visual = "S",
        visual_line = "gS",
        delete = "ds",
        change = "cs",
    },

Otherwise you'll have no keymaps whatsoever.

SidOfc commented 2 years ago

@kylechui I can confirm this works indeed!

Additionally I've actually made some changes to my file explorer plugin. Using nowait = true in its mappings. This allows me to use the regular require('nvim-surround').setup() without any problems.

Thank you very much for your time researching and providing solutions, it is much appreciated! Your plugin is absolutely wonderful and in my opinion it's a step up from tpope/vim-surround since it can also highlight what will be surrounded :)

This can certainly remain closed, have a good one!