Jump to the nearest pair/delimiter

[spywhere]

When trigger cs or ds on vim-surround, the plugin will attempt to find the nearest matching pair/delimiter for replacement/deletion. It would be great if we could have that in nvim-surround as well.

For example, with the snippet below with cursor at the beginning of the line. Trigger cs({ would turn that line into table.method{ 'some basic string' } with the cursor on {.

Snippet

table.method('some basic string')

Reproducible configurations (run with nvim -u <config.lua>)

-- plugins will be installed to the cache directory
local plugin_home = vim.fn.stdpath('cache') .. '/temp'
local config_home = vim.fn.stdpath('config')
local plug_path = config_home .. '/lua/plug.lua'
local plug_url = 'https://raw.githubusercontent.com/spywhere/plug.nvim/main/plug.lua'

if vim.fn.filereadable(vim.fn.expand(plug_path)) == 0 then
  if vim.fn.executable('curl') == 0 then
    -- curl not installed, skip the config
    print('cannot install plug.nvim, curl is not installed')
    return
  end
  vim.cmd(
    'silent !curl -fLo ' .. plug_path .. ' --create-dirs ' .. plug_url
  )
end

vim.opt.runtimepath:append(plugin_home)

local plug = require('plug')

plug.setup {
  plugin_dir = plugin_home,
  extensions = {
    -- also perform automatic installation for vim-plug and missing plugins
    plug.extension.auto_install {},
    plug.extension.config {}
  }
}

{
  'kylechui/nvim-surround',
  config = function ()
    require('nvim-surround').setup {}
  end
}

''

[kylechui]

Ah, so this is something that I actually implemented "as a feature, not a bug". I thought it was a bit strange at first that the cursor would always "jump" to the next pair instead of only triggering when the cursor sat inside the pair. I think what you're asking for might have some conflict with the following case when using q for quote aliasing:

Buffer: local str = "This is a string with a 'quote' inside of it"
Cursor:                        ^

If I type csqb, what should happen? Should the plugin modify the "true" surrounding quotes to become local str = (This is a string with a 'quote' inside of it), or jump first and change it to local str = "This is a string with a (quote) inside of it"? Implementation-wise at least, adding the "feature" of jumping would be easiest with the latter (I think it would literally only delete code), but in my head it makes more sense with the former. I'm not ruling this out for now, but I would like to see more people weigh in with their thoughts before I make any moves

[spywhere]

Personally, I think it could benefit from a or i modifier (not sure with the actual term) as well. But more opinions would be best here as well.

Given that the cursor is inside a double quote but not a single quote...

aq as in csaqb/ daq would

Turn: local str = "This is a string with a 'quote' inside of it"
Into: local str = (This is a string with a 'quote' inside of it)

iq as in csiqb/ diq would

Turn: local str = "This is a string with a 'quote' inside of it"
Into: local str = "This is a string with a (quote) inside of it"

[kylechui]

Hmmm... this would be quite interesting, but I think would result in a lot more work on my end to discern what's "inner" vs "around", but I'll try and keep this in mind for future improvements after I finish squashing the endless number of bugs that keep seeming to crop up (if that ever happens haha). I guess as a short mental note to myself, I'll write that one possible implementation would be to jump backwards and forwards to the nearest delimiting char, for each quote type, and then use operatorfuncs to find the "nearest one", while using getchar() to determine if inner/outer parsing is necessary.

[kylechui]

I think the main thing keeping me from proceeding with this idea right now is the "ambiguity" regarding how exactly things should work, and what "rules" are baked into nvim-surround. For example, to answer the question "should nvim-surround jump to the nearest pair", what does that mean exactly? Is it always the closest pair forwards? Or maybe backwards as well? Does jumping to that pair break any existing ideas of how nvim-surround should work (e.g. my comment here). As of right now, nvim-surround maintains the relatively sane/unambiguous stance of "if the cursor is not inside (inclusively) the delimiter pair, then don't consider it", and I'm not 100% sure if that should change. I'm definitely down to hear more feedback from the community to see if there are ways to have "sane defaults" with jumping though!

[spywhere]

I'm not sure if you're aware that tpope/vim-surround also have the same feature I'm requesting here?

Let me clarify what I mean by nearest, I don't expecting it to be smart with all the context (e.g. to know that it should change ' in let x = 'hello ' .. text when the cursor is on the word text). I believe what vim-surround did in this case is just to find the nearest pair/delimiter that is closer to the cursor in a forward direction.

I was requesting for a feature parity that I find lacking from vim-surround, which I found useful to be baked into nvim-surround as well. Hence this issue.

Here's the configuration for vim-surround if you need to play around with some edges cases.

-- plugins will be installed to the cache directory
local plugin_home = vim.fn.stdpath('cache') .. '/temp'
local config_home = vim.fn.stdpath('config')
local plug_path = config_home .. '/lua/plug.lua'
local plug_url = 'https://raw.githubusercontent.com/spywhere/plug.nvim/main/plug.lua'

if vim.fn.filereadable(vim.fn.expand(plug_path)) == 0 then
  if vim.fn.executable('curl') == 0 then
    -- curl not installed, skip the config
    print('cannot install plug.nvim, curl is not installed')
    return
  end
  vim.cmd(
    'silent !curl -fLo ' .. plug_path .. ' --create-dirs ' .. plug_url
  )
end

vim.opt.runtimepath:append(plugin_home)

local plug = require('plug')

plug.setup {
  plugin_dir = plugin_home,
  extensions = {
    -- also perform automatic installation for vim-plug and missing plugins
    plug.extension.auto_install {},
    plug.extension.config {}
  }
}

'tpope/vim-surround'

''

[kylechui]

Ah I see now, I think this is a worthy feature to implement then. However since vim-surround doesn't have aliasing, going back to the example:

Buffer: local str = "This is a string with a 'quote' inside of it"
Cursor:                        1                              2

Do you think dsq should target the double or single quotes? I know you mentioned the i/a modifier but my current thought process is to simply always choose the "left-most quote that can be found by forward-searching". In this particular case, running dsq at both positions 1 and 2 would delete the double quotes, but if the double quotes didn't exist, then position 1 would forward-search to delete the single quotes, and position 2 would effectively be a NOOP. I think following this rule would provide default feature-matching with vim-surround when not aliasing, e.g. ds" or ds', while providing a decent default for nvim-surround only cases

[spywhere]

I think your thought process is sounded in my opinion. I would expecting the same outcome as yours if the alias is work that way.

I believe we could go with a simplest implementation to give the same feature parity. Then on certain cases that vim-surround doesn't covered, we could have those as an enhancement to this feature as well (as those would be a feature of nvim-surround itself).

[kylechui]

Sounds good to me, I'll see if I can start work on this sometime tomorrow

[kylechui]

Hey @spywhere, I've just made a new commit to branch jump-to-delimiter that should hopefully address this issue. The heuristic that's currently in place is the following:

• Always choose the pair that starts closest to the cursor (forward-searching included), but prioritize pairs that actually contain the cursor

What this means in practice:

local str = "This string has 'a few' nested quotes" -- Some other content
-----------------------------_______---------------**********************

In the above example, running dsq will:

• If run at a dashed zone, delete the double quotes
• If run at an underscored zone, delete the single quotes
• If run at an asterisk zone, do nothing

Another way to think about the heuristic (that might be more convoluted, but is how the code is implemented):

• If the cursor does not reside inside any pair, choose the one that starts as left-most as possible (but still after the cursor)
• If the cursor resides inside one (more more) pairs, choose the one that starts as right-most as possible (but still contains the cursor)

As always, I'm interested to hear any feedback that you (or anybody else that might be reading this) might have!

Edit: Pinging @andrewferrier and @rammiah since they have showed interest in this enhancement

[spywhere]

Hey @kylechui, thanks for such a quick response and implementation. So far the behavior seem great for me.

However, I notice one thing. For the following example

if vim.fn.filereadable(vim.fn.expand(plug_path)) == 0 then

At any point on the line, I cannot perform cs({ or cs(}. Although cs){ and cs)} is working fine. Since the default seem to include both ( and ) pair, I believe this is something that is broken by this feature?

Here's config used for testing

-- plugins will be installed to the cache directory
local plugin_home = vim.fn.stdpath('cache') .. '/temp'
local config_home = vim.fn.stdpath('config')
local plug_path = config_home .. '/lua/plug.lua'
local plug_url = 'https://raw.githubusercontent.com/spywhere/plug.nvim/main/plug.lua'

if vim.fn.filereadable(vim.fn.expand(plug_path)) == 0 then
  if vim.fn.executable('curl') == 0 then
    -- curl not installed, skip the config
    print('cannot install plug.nvim, curl is not installed')
    return
  end
  vim.cmd(
    'silent !curl -fLo ' .. plug_path .. ' --create-dirs ' .. plug_url
  )
end

vim.opt.runtimepath:append(plugin_home)

local plug = require('plug')

plug.setup (
  plugin_dir = plugin_home,
  extensions = {
    -- also perform automatic installation for vim-plug and missing plugins
    plug.extension.auto_install {},
    plug.extension.config {}
  }
)

{
  'kylechui/nvim-surround',
  options = {
    branch = 'jump-to-delimiter'
  },
  config = function ()
    require('nvim-surround').setup {}
  end
}

''

[kylechui]

Ah so that was a recent change made to address #20, where using { or ( "respects whitespace". In the example you gave, there exists no whitespace after the (, so it can't be replaced there (the default mappings include whitespace for "opening" pairs and no whitespace for "closing" pairs). I would recommend either using csb{ or cs){, or overriding the default values with

require("nvim-surround").setup({
    delimiters = {
        pairs = {
            ["("] = { "(", ")" }
            -- Etc. for {, [, < if you would like the same behavior
        }
    }
})

Note: You can also do this on a per-buffer basis if you would like by calling buffer_setup instead.

[spywhere]

I see, that make sense. However to add some note, vim-surround does support cs({ even though ( does not have to have a whitespace. So that vim.fn.expand(plug_path) could become vim.fn.expand{ plug_path }. So the whitespace is respected during a replacement but not searching, but I think that would be outside of this issue's scope.

All and all, I think the feature I'm requesting is fulfilled my needs. Feel free to close it, I'm looking forward to using this very soon. Thanks! 🎉

[kylechui]

Haha I was literally just about to reinstall vim-surround to check if it supported this sort of "dynamic whitespace surround". I'll close this issue, and hopefully fix it so you can use cs({ shortly.

[williamgoulois]

@kylechui First thanks for this plugin I just installed it and I'm a previous user of tpope's plugin. I tested with your jump-to-delimiter branch and it works according to the spec. Instead of a NOOP after the surrounding i would love for it to work backwards (on first occurence). So your example would become this :

local str = "This string has 'a few' nested quotes" -- Some other content
-----------------------------_______-----------------------------------

Btw I saw that jumping is not consistent : When you add a new surrounding the cursor jumps to the first occurence of the new surround. When you change or delete an existing surrounding the cursor stays in place. Is this behavior wanted ?

[kylechui]

@williamgoulois What do you propose for the scenario where the cursor sits between two pairs of quotes then? To always prefer forward searching before reverse searching? Ideally I'd like to preserve feature parity with vim-surround so this might work (if vim-surround never reverse searches)

As for the inconsistent jumps, oops I'll get to patching that as well haha

[williamgoulois]

@kylechui After i sent my message i just thought of this case too 🙂 I think that the most logic way of doing this is prefer forward over backward searching. In terms of logic if you are not inside a surround and there is not surrounding starting after cursor just search backwards. I'm not sure if this is in parity with vim-surround though you might want to add this as an opt-in that can be activated with setup config maybe ?

Nice 😁

[kylechui]

@williamgoulois That sounds good to me; although I think in this particular case no setup option should be necessary, as it seems relatively straightforwards to have the priority go: contains cursor, is after cursor, is before cursor

I think the most important thing here is to not do anything "unexpected", and users of vim-surround will in the worst case not know about reverse searching, but otherwise have the plugin behave "as intended" for them

[williamgoulois]

@kylechui I totally agree if you use those keystrokes you really want to update surroundings so you expect an operation. Operation is often better than NOOP 🙂 This is unpexpected but in sense that it's better, more intelligent !

Moreover I often use this just after arriving in new line using j or k so i'm not always directly on top of the thing i want to change and it may be backwards 😉

[kylechui]

@williamgoulois Do you mind testing out the latest commit on jump-to-delimiter? It should implement both reverse-jumping, as well as fix the issue where the cursor doesn't jump to the correct location. Thanks!

[kylechui]

One thing that I've found that might be confusing to some people is what happens when one is deleting quotes---Forward searching only deletes quotes on the same line, so maybe reverse-searching should only do so as well? Otherwise this is a weird edge case that pops up:

local str = "some text"
-- A comment
local tab = { "a", "table" }

Positioning the cursor over the comment and trying ds" deletes the quote on the first line, which doesn't really seem like it should be intended behavior. If I remember correctly vim-surround should do nothing in this particular use case, so maybe nvim-surround should as well. However in the case

local str = "some text" -- A comment
local tab = { "a", "table" }

I think it makes sense to delete the quotes on the first line if dsq is run over the comment. I would be open to hearing more thoughts though!

[williamgoulois]

@kylechui You got it working pretty fast ! 🎉

Forward searching only deletes quotes on the same line, so maybe reverse-searching should only do so as well?

Yes i think so !

I found another weird behavior when you are in between two surroundings of same type. I dunno if it is hard to fix. https://user-images.githubusercontent.com/37271970/178130489-bffca46a-5946-413b-bca7-b6aed8559dd7.mov

[kylechui]

@williamgoulois The underlying functionality for (n)vim-surround is to use the builtin operatorfunc to get selections from motions. Right now the builtin behavior for the " textobject is to select the immediately surrounding two quotes on the current line, e.g. if you try va" you will see that it selects the same two "incorrect" quotes. If I were to implement this, it would require a bit more effort, but I'll keep it in mind for a future update

[williamgoulois]

@kylechui Ok i understand ! Maybe the better way would be to totally reimplement this as a treesiter plugin 😆 Thanks a lot for your fixes I totally made the switch to your plugin 🎉

Feel free to close this issue !

[kylechui]

I had considered using Treesitter before, but I don't really want to make that a requirement for use... yet (although perhaps that's something that can be added as an option in the future, e.g. detect if TS is enabled for the current buffer :thinking:)

Edit: I'll re-merge/close this issue once I'm able to get the quote edge case working

[williamgoulois]

I know that some plugins like https://github.com/lukas-reineke/indent-blankline.nvim#with-context-indent-highlighted-by-treesitter have an option to enable TS functionality maybe it can be a source of inspiration ? 🤔

I think that most dev use TS for the languages they code in (and i mean here that they have to modify) and TS is really one of the big differences btw vim and neovim 🙂

[kylechui]

Do you mind opening a new issue for TS support then? That does seem like it would be quite a good feature to have on board.