Better which-key integration

[Isrothy]

First of all, thank you for your great plugin!

Checklist

• Have you read through :h nvim-surround to see if there might be any relevant information there?

Is your feature request related to a problem? Please describe.
If possible, provide a sample buffer/keymap sequence, along with the expected result.
no

Describe the solution you'd like
A clear and concise description of what you want to happen.
After I stroke, for example, ds, whick-key shows suggestions like ( -> delete surrounding (), f -> delete surrounding function, etc.

Additional context
Add any other context or screenshots about the feature request here.

[kylechui]

I don't believe that this is possible at the moment, since nvim-surround doesn't actually define a set of keymaps dsf, dsb, etc. Rather, it defines only the ds operator which then takes in some extra character as input. Thus I don't think it would be feasible to have custom descriptions for all of ds(, ds), ... without rewriting the configuration, and I don't think it would be scalable either. Do feel free to chime in if you have some ideas that I'm not aware of that could implement this without incurring significant cost.

[firubat]

@kylechui If I understand correctly, which-key has a notion of "groups" which actually fits really well with operators. E.g when you press "d", you get a group denoted "Delete" and in it you have the different motions that can follow.
Similarly it would be great that when you hit the operator for add/delete/change surrounding you will get a group of the options you now have which would be fed by the surrounds table.

e.g. in the case of delete, after pressing ds the which-key popup will have "Delete surrounding" at the bottom, and the the list of defined surroundings that you can delete.

So there's no need for custom description for each mappings - but a description for each operator and perhaps a way to define descriptions for surroundings.

[kylechui]

I messed around with it a little bit and don't believe that which-key plays very nicely with how nvim-surround handles these operations, i.e. for delete and change operations it actually uses g@l as the motion, and for adding a surround ys → g@. Because of this, I don't think I can make the description for the operators properly. I also don't normally use which-key, so there might be something that I'm missing here; here's the config I was messing with:

return {
    [1] = "folke/which-key.nvim",
    event = "VeryLazy",
    config = function()
        require("which-key").setup({})
        require("which-key").add({
            preset = true,
            mode = { "n" },
            { "ys", desc = "You surround" }, -- Doesn't generate anything
            { "g@", desc = "You surround" }, -- Generates the message, but will activate for any g@ operation
        })
    end,
}

[firubat]

OK, thanks for trying.
I did find the following:

• https://github.com/roobert/surround-ui.nvim
• https://www.reddit.com/r/neovim/comments/1d0ynca/whichkey_mappings_for_nvimsurround/

Maybe something there would be a solution, although it seems that the actually redefine the mappings to by prefixed with <leader>

[gregorias]

For the visual mode surrounds I did the following to good effect:

require("nvim-surround").setup({ keymaps = { visual = false } })
local wk = require("which-key")
wk.add({
	mode = "x",
	[1] = {
		[1] = "S",
		-- Force showing the keybindings for nvim-surround.
		[2] = function()
			wk.show({ keys = "S" })
		end,
		group = "+Surround",
	},
	[2] = { [1] = "S(", [2] = "<Plug>(nvim-surround-visual)(", desc = "( … )" },
	[3] = { [1] = "S)", [2] = "<Plug>(nvim-surround-visual))", desc = "(…)" },
	[4] = { [1] = "S{", [2] = "<Plug>(nvim-surround-visual){", desc = "{ … }" },
	[5] = { [1] = "S}", [2] = "<Plug>(nvim-surround-visual)}", desc = "{…}" },
	[6] = { [1] = "S<", [2] = "<Plug>(nvim-surround-visual)<", desc = "< … >" },
	[7] = { [1] = "S>", [2] = "<Plug>(nvim-surround-visual)>", desc = "<…>" },
	[8] = { [1] = "S[", [2] = "<Plug>(nvim-surround-visual)[", desc = "[ … >" },
	[9] = { [1] = "S]", [2] = "<Plug>(nvim-surround-visual)]", desc = "[…]" },
	[10] = { [1] = "S'", [2] = "<Plug>(nvim-surround-visual)'", desc = "'…'" },
	[11] = { [1] = 'S"', [2] = '<Plug>(nvim-surround-visual)"', desc = '"…"' },
	[12] = { [1] = "S`", [2] = "<Plug>(nvim-surround-visual)`", desc = "`…`" },
	[13] = { [1] = "Si", [2] = "<Plug>(nvim-surround-visual)i", desc = "left…right" },
	[14] = { [1] = "St", [2] = "<Plug>(nvim-surround-visual)t", desc = "<left>…</right>" },
	[15] = { [1] = "Sf", [2] = "<Plug>(nvim-surround-visual)f", desc = "foo(…)" },
	[16] = { [1] = "Sa", [2] = "<Plug>(nvim-surround-visual)a", desc = "<…>" },
	[17] = { [1] = "Sc", [2] = "<Plug>(nvim-surround-visual)c", desc = "```…```" },
})

The result when pressing S in the visual mode:

I think the same think can be done for deletes. I don’t know how to do ys, because it first accepts a motion and only then the surround, which is not idiomatic (operation, i.e., the surround, should come first).

[firubat]

@gregorias nice, it basically works, but I sometimes get the following error:

Recursion detected.
Are you manually loading which-key in a keymap?
Use `opts.triggers` instead.
Please check the docs.

Is there a reason you use the wk.show inside the keymap and not simply outside?

(BTW could you reference me an explanation about the syntax you use with all the numbers? I'm a bit of a noob wrt lua...)

[gregorias]

Is there a reason you use the wk.show inside the keymap and not simply outside?

Nothing other than it’s the only way I was able to make it work, but I was unfamiliar of the trigger functionality. If someone figures out the proper way to trigger WK on S, I’ll welcome it.

So I guess, the answer is, instead of adding the wk.show() keymap, one should add S to opts.triggers during WK’s setup.

triggers = {
  -- Prefixes for nvim-surround.
  { [1] = "S", mode = { "x" } },
  { [1] = "ds", mode = { "n" } },
}

(BTW could you reference me an explanation about the syntax you use with all the numbers? I'm a bit of a noob wrt lua...)

It’s a long form for naming table keys (SO). I just have a linter that complains if I don’t consistently name my keys.

[gregorias]

@kylechui, you wrote that WK integration may not be possible in 2023, but could you now reconsider and reopen this task? In the posts above I’ve shown a reliable way to add support for surround-visual. You can have a similar support for surround-delete. I believe it’s possible to have Which Key support for all functionalities.

Here are some additional notes on this that could help:

1. Instead of hardcoding Which Key keymaps, we can use the expand property added in WKv3. We can generate WK options on the fly from Surround’s internal config (see also 2.).
2. The labels for Which Key can be generated from a static user_options.add table, e.g., add = { '{', '}' } could easily generate {…} via string concatenation. Function-based surrounds lose that capability (as the function may block on user action), so perhaps it’s worth it a label field to the config for cases where add is such a function.
3. The ys operator is tricky, because it first accepts movement and then it accepts the surround key. I’d switch the order of parameters, so that then it’s possible to integrate with Which Key: you open the Which Key list for surround keys on ys and then you trigger the operator to accept movement. Switching the order of operations would also make it more idiomatic as movement should come last.

[firubat]

@gregorias Have you managed to set the menus for ds or cs? For me they don't work.
About 3. I don't think it's fair to ask the author to change the innerworkings of a plugin for this...

[gregorias]

Have you managed to set the menus for ds or cs? For me they don't work.

The ds bindings work fine. I haven’t set up cs bindings, but I see no reason why they wouldn’t work. A combination of normal bindings + expand should do the trick.

About 3. I don't think it's fair to ask the author to change the innerworkings of a plugin for this...

I don’t see how fairness got anything to do here. I shared a few ideas that can help, I’m not the owner’s boss. They can do whatever they wish.

[firubat]

The ds bindings work fine.

I couldn't get the triggers thing to work, so played with it, and seems that the problem for me is that ds needs to have some rhs for things to work, even if the rhs is "empty". So the following works

{
          mode = "n",
          [1] = {
            "ds",
            "",
            desc = "Surrounding",
          },
          [2] = { [1] = "ds(", [2] = "<Plug>(nvim-surround-delete)(", desc = "( … )" },
          [3] = { [1] = "ds)", [2] = "<Plug>(nvim-surround-delete))", desc = "(…)" },
          [4] = { [1] = "ds{", [2] = "<Plug>(nvim-surround-delete){", desc = "{ … }" },
          [5] = { [1] = "ds}", [2] = "<Plug>(nvim-surround-delete)}", desc = "{…}" },
          [6] = { [1] = "ds<", [2] = "<Plug>(nvim-surround-delete)<", desc = "< … >" },
          [7] = { [1] = "ds>", [2] = "<Plug>(nvim-surround-delete)>", desc = "<…>" },
          [8] = { [1] = "ds[", [2] = "<Plug>(nvim-surround-delete)[", desc = "[ … >" },
          [9] = { [1] = "ds]", [2] = "<Plug>(nvim-surround-delete)]", desc = "[…]" },
          [10] = { [1] = "ds'", [2] = "<Plug>(nvim-surround-delete)'", desc = "'…'" },
          [11] = { [1] = 'ds"', [2] = '<Plug>(nvim-surround-delete)"', desc = '"…"' },
          [12] = { [1] = "ds`", [2] = "<Plug>(nvim-surround-delete)`", desc = "`…`" },
          [13] = { [1] = "dsi", [2] = "<Plug>(nvim-surround-delete)i", desc = "left…right" },
          [14] = { [1] = "dst", [2] = "<Plug>(nvim-surround-delete)t", desc = "<left>…</right>" },
          [15] = { [1] = "dsf", [2] = "<Plug>(nvim-surround-delete)f", desc = "foo(…)" },
          [16] = { [1] = "dsa", [2] = "<Plug>(nvim-surround-delete)a", desc = "<…>" },
          [17] = { [1] = "dsc", [2] = "<Plug>(nvim-surround-delete)c", desc = "```…```" },
        },

I don’t see how fairness got anything to do here.

Probably "fair" isn't the right word. But nevermind, maybe I read your suggestion differently.

[kylechui]

I'm not particularly interested in doing all of this setup, especially because it doesn't seem very extensible to other people's custom surrounds. I'd much rather there be a discussion post where people put these "integrations with other plugins", rather than trying to manage it all myself in the core of the plugin. As for changing the default behavior of ys, that will never happen since it's subjective, and I imagine most people (including myself) have grown accustomed to how the OG vim-surround does things.

I also don't personally use which-key, which makes this even more troublesome for me to verify that things are working, etc.

IMO which-key integration is a "nice-to-have" and does not affect the core of the plugin, so to consider adding it I would want:

• It should automatically work with any custom surround the user defines
• It should not consist of hard-coding every possible surround pair

As always, comments + PRs welcome

[gregorias]

I got around to proposing a WK integration in #447. It uses a WK plugin and I think it satisfies your constraints:

• WK implementation is mostly decoupled from the rest of the plugin, so mistakes there shouldn’t interfere with core logic. The only touchpoints are: introduction of labels to surrounds (optional) and a hook it get_char.
• Works automatically with custom surrounds.
• Doesn’t require hard-coding surround pairs.