Skip to content

feat: add WK plugin#447

Closed
gregorias wants to merge 1 commit into
kylechui:mainfrom
gregorias:feature/neo
Closed

feat: add WK plugin#447
gregorias wants to merge 1 commit into
kylechui:mainfrom
gregorias:feature/neo

Conversation

@gregorias

Copy link
Copy Markdown
Contributor

I have way too many surrounds to rememeber them, and having hints is a must. This commit adds an option WK plugin that shows surround hints.

ADRs:

  • Used the WK plugin functionality instead of WK expand, because it fits the use-case better: we only have a single-level of hints, we can keep most of the added functionality/complexity in a single module.
  • Used the hardcoded "⌨S" keymap: WK requires a keymap. "⌨" as the prefix practically guarantees no conflicts.
  • Requiring providing labels instead of automatically generating them: there are some corner-cases that automation wouldn't be able to solve, e.g., i. Better to have one mechanism for labels than have manual labels and automatic labels.

Known limitations:

  • This plugin can not accept arbitrary characters. WK requires registering all possible answers, so I decided against it to not pollute the popup (there’s i for arbitrary surrounds).
  • This plugin shows all possible surrounds regardless of whether a surround is available. It makes the implementation simpler, but could be worked on if desired.

I have way too many surrounds to rememeber them, and having hints is a
must. This commit adds an option WK plugin that shows surround hints.

ADRs:

- Used the WK plugin functionality instead of WK expand, because it fits
  the use-case better: we only have a single-level of hints, we can
  keep most of the added functionality/complexity in a single module.
- Used the hardcoded "⌨S" keymap: WK requires a keymap. "⌨" as the
  prefix practically guarantees no conflicts.
- Requiring providing labels instead of automatically generating them:
  there are some corner-cases that automation wouldn't be able to solve,
  e.g., `i`. Better to have one mechanism for labels than have manual
  labels and automatic labels.

Known limitations:

- This plugin can not accept arbitrary characters. WK requires
  registering all possible answers, so I decided against it to not
  pollute the popup (there’s `i` for arbitrary surrounds).
- This plugin shows all possible surrounds regardless of whether a
  surround is available. It makes the implementation simpler, but
  could be worked on if desired.
@gregorias gregorias mentioned this pull request May 24, 2026
1 task
@gregorias

Copy link
Copy Markdown
Contributor Author

Related ticket (#205).

@kylechui

Copy link
Copy Markdown
Owner

After looking at things a little bit closer, is it possible to factor this code out into a separate plugin? It seems like while small, it still does affect some of the core behavior of the plugin (i.e. by modifying the get_char behavior), and I would prefer not to have that. That would also allow the maintainer of that to add additional functionality, e.g. by supporting the limitations that you describe in the feature description.

@gregorias

Copy link
Copy Markdown
Contributor Author

Sounds fair that we make it more modular and put it into a separate plugin. There are two issues I’d like to discuss:

It would be very beneficial if nvim-surround (NS) had label in its surround data type that the plugin could then retrieve. Otherwise, the plugin would have to keep its own global/buffer surround state with labels, and make sure it stays in sync with NS.
Neovim keymaps come with the desc field (that WK reuses). How about we add labels to surrounds? That should be a minimal increase in maintenance burden.

The plugin will have to monkey patch get_char, which is fine IMO, but if you’d like to provide some future-compatible way of hooking into the input method, it could be desirable.

LMK what you think.

@kylechui

Copy link
Copy Markdown
Owner

Sorry, what is the difference between a label and a description? I feel like having the which-key integration define the labels shouldn't be too much more work, given that the default surrounds haven't really changed in ~4 years, and I don't expect them to change in the forseeable future

@gregorias

Copy link
Copy Markdown
Contributor Author

Sorry, what is the difference between a label and a description?

They are two words for the same thing. I’ll use “description” now on.

I meant that the current relationship between Which-Key and Neovim keymaps is that Neovim keeps the state of all keymaps together with their descriptions. Which-key doesn’t have a separate table for descriptions.

graph LR
  subgraph neovim
    keymaps[(Keymaps with descriptions)]
  end

  subgraph which-key
    key-tree[(Key tree)]
  end

  which-key --> keymaps
Loading

Note

Which-key happens to post-process keymaps into a TRIE-like structure for WK’s key traversal, but that’s irrelevant for our discussion.

I believe that a similar approach where Nvim-surround maintains descriptions (i.e., the approach used in this PR) would be best for the WK nvim-surround plugin.

What if the WK plugin keeps its own descriptions?

I feel like having the which-key integration define the labels shouldn't be too much more work,

I don’t think so. I think it will be meaningfully more complicated and fragile to keep labels it the WK plugins. Let’s walk through implications of such an architecture.

The WK plugin (WKP) will have to have its own tables that map chars to surrounds. Nvim-surround (NS) has a global table and buffer tables so WKP will have to have such tables as well in order to accurately represent descriptions. The big issue with this is that we have a split-brain problem. WKP will have to make sure that its state is consistent with NS. All those global and per-buffer tables across both plugins will have to have the same chars (keys) to provide consistent experience.

graph LR
  subgraph nvim-surround
    ns-surrounds[(char to surrounds)]
  end

  subgraph wk-plugin
    wkp-surrounds[(char to descriptions)]
  end

  ns-surrounds <-.->|must keep consistent| wkp-surrounds
Loading

A globally optimal solution would keep the necessary state entirely within NS.

Then there’s an issue of how both tables are filled. Users can provide their own surround to NS. WKP will have to have its own API for providing descriptions, and it would either mean that:

  1. The user makes two calls: once to add a surround to NS and once to add its description to WKP.
  2. WKP provides a wrapper that adds a surround to NS on behalf of the user.

Both options make the plugin more burdensome, because they add additional burdens on the user (additional APIs).

LMK if this is still unclear.
Whatever you decide, I’ll adapt.

@kylechui

kylechui commented Jun 6, 2026

Copy link
Copy Markdown
Owner

I think I am OK with adding a label field with type string | nil to each of the existing surrounds, but probably no more than that. I would strongly prefer the vast majority of the logic to remain in some external plugin. Would you still need some override for the input function in order to do some which-key registration? I think that might be going a bit far; ideally my plugin shouldn't be aware that yours exists.

@gregorias

Copy link
Copy Markdown
Contributor Author

I think I am OK with adding a label field with type string | nil…

Sounds good, I think. I will now fork this PR off into a separate plugin to make sure that those labels are the only needed feature. I will be back with either label-only PR or further comments.

@gregorias

Copy link
Copy Markdown
Contributor Author

OK, I got the plugin ready at https://github.com/gregorias/nvim-surround-wk and the label PR in #449. I’m closing this PR.

@gregorias gregorias closed this Jun 8, 2026
gregorias added a commit to gregorias/nvim-surround that referenced this pull request Jun 8, 2026
Add labels to surrounds. This will be useful to plugins that show key
hints. For discussion, see
kylechui#447.
kylechui pushed a commit that referenced this pull request Jun 8, 2026
Add labels to surrounds. This will be useful to plugins that show key
hints. For discussion, see
#447.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants