Skip to content

fix(mini): disable mini.surround by default to preserve native S key behavior #1553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SetsuikiHyoryu
Copy link

I have confirmed the source code of mini.surround (mini.nvim/lua/mini/surround.lua). Its setup function looks like this:

MiniSurround.setup = function(config)
  -- ...

  -- Setup config
  config = H.setup_config(config)

  -- Apply config
  H.apply_config(config)

  -- ...
end

If the user does not modify config, setup_config will use the default_config:

H.setup_config = function(config)
  H.check_type('config', config, 'table', true)
  config = vim.tbl_deep_extend('force', vim.deepcopy(H.default_config), config or {})

  -- ...

  return config
end

In default_config.mappings, multiple mappings start with the character s:

MiniSurround.config = {
  -- ...

  -- Module mappings. Use `''` (empty string) to disable one.
  mappings = {
    add = 'sa',          -- Add surrounding in Normal and Visual modes
    delete = 'sd',       -- Delete surrounding
    find = 'sf',         -- Find surrounding (to the right)
    find_left = 'sF',    -- Find surrounding (to the left)
    highlight = 'sh',    -- Highlight surrounding
    replace = 'sr',      -- Replace surrounding
    update_n_lines = 'sn', -- Update `n_lines`

    suffix_last = 'l',   -- Suffix to search with "prev" method
    suffix_next = 'n',   -- Suffix to search with "next" method
  },

  -- ...
}

H.default_config = vim.deepcopy(MiniSurround.config)

These mappings are then applied in the apply_config function called after setup:

H.apply_config = function(config)
  MiniSurround.config = config

  local expr_map = function(lhs, rhs, desc) H.map('n', lhs, rhs, { expr = true, desc = desc }) end
  local map = function(lhs, rhs, desc) H.map('n', lhs, rhs, { desc = desc }) end

  --stylua: ignore start
  -- Make regular mappings
  local m = config.mappings

  expr_map(m.add,     H.make_operator('add', nil, true), 'Add surrounding')
  expr_map(m.delete,  H.make_operator('delete'),         'Delete surrounding')
  expr_map(m.replace, H.make_operator('replace'),        'Replace surrounding')

  -- ...
end

The Neovim documentation on mappings (nvim/runtime/doc/map.txt) in the map-ambiguous section states:

“When two mappings start with the same sequence of characters, ... Vim is waiting for another character”:

    							*map-ambiguous*
When two mappings start with the same sequence of characters, they are
ambiguous.  Example: >
	:imap aa foo
	:imap aaa bar
When Vim has read "aa", it will need to get another character to be able to
decide if "aa" or "aaa" should be mapped.  This means that after typing "aa"
that mapping won't get expanded yet, Vim is waiting for another character.
If you type a space, then "foo" will get inserted, plus the space.  If you
type "a", then "bar" will get inserted.

I don't need the features this plugin provides, and it significantly disrupts the feel of my normal workflow.

I also see the #1328.

Therefore, I believe this plugin should be disabled by default, with a note explaining that its default mappings interfere with Neovim’s native S key behavior. Users should enable it consciously, understanding this limitation, or customize the mappings to avoid conflicts.

I believe beginner-friendly configurations should not include hidden side effects—especially those that override native key behaviors. If there's a strong reason this plugin must be enabled by default, please keep the warning comment to inform users.

@SetsuikiHyoryu SetsuikiHyoryu changed the title fix(mini): Disable mini.surround by default to preserve native S key behavior fix(mini): disable mini.surround by default to preserve native S key behavior May 19, 2025
@ro0gr
Copy link

ro0gr commented May 19, 2025

I don't remember the why, but I think that's why I ended up switching to "kylechui/nvim-surround". In my setup, it does not seem to have the issue you are describing.

If we do decide to drop the "mini.surround", I'd suggest installing mini.ai as a standalone package instead of pulling in the entire suite unnecessarily. Maybe it's just me, but I find it a bit annoying to see a bunch of unrelated mini updates when doing the "Lazy Sync", especially when I'm only using a single module from the collection.

@SetsuikiHyoryu
Copy link
Author

SetsuikiHyoryu commented May 19, 2025

I don't remember the why, but I think that's why I ended up switching to "kylechui/nvim-surround". In my setup, it does not seem to have the issue you are describing.

@ro0gr

Thank you for the introduction.

I took a look at the Usage section of nvim-surround:

https://github.com/kylechui/nvim-surround?tab=readme-ov-file#rocket-usage

It seems that by default, it uses keys like y, d, and c, which already have built-in timeout behavior in Neovim. The semantics of the operations are also intuitive and easy to understand.

I think this plugin takes a more reasonable approach.

@szechp
Copy link
Contributor

szechp commented May 20, 2025

I don't remember the why, but I think that's why I ended up switching to "kylechui/nvim-surround". In my setup, it does not seem to have the issue you are describing.

@ro0gr

Thank you for the introduction.

I took a look at the Usage section of nvim-surround:

https://github.com/kylechui/nvim-surround?tab=readme-ov-file#rocket-usage

It seems that by default, it uses keys like y, d, and c, which already have built-in timeout behavior in Neovim. The semantics of the operations are also intuitive and easy to understand.

I think this plugin takes a more reasonable approach.

we could do default mappings like the ones nvim-surround has? the proposal you make would mean installing another plugin?

i personally also like the lazyvim keybindings, its the same, but starts with g (example: gsa is for adding surroundings), which also fixes the doube-assignment with s.

@SetsuikiHyoryu
Copy link
Author

@szechp

we could do default mappings like the ones nvim-surround has? the proposal you make would mean installing another plugin?

i personally also like the lazyvim keybindings, its the same, but starts with g (example: gsa is for adding surroundings), which also fixes the doube-assignment with s.

If change the plugin, users who are already using mini.surround will encounter unfamiliar keybindings with other plugins, and the same goes for changing the key mappings. On the other hand, if we simply make the plugin disabled by default, those who actually use its functionality can just re-enable it, while those who never used it in the first place won't feel like they've lost any functionality—in fact, they might feel that the s key behaves more normally.

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.

3 participants