kylechui / nvim-surround

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

Use surrounds as textobjects (like `vim-sandwich`) #135

Closed haoming-li-ling closed 3 months ago

haoming-li-ling commented 2 years ago

vim-sandwich has the functionality to turn user-define surrounds into textobjects through the recommended mappings as<char> and is<char>; suppose I have defined the char x for the delimiters foo and bar; then, for fooSOME_TEXTbar, dsx would result in SOME_TEXT, dasx would result in the entire phrase being deleted, and disx would result in foobar. Since we are already providing the find and delete patterns for user-defined surrounds, I think it would not be too difficult to implement this textobject functionality. This is the one vim-sandwich feature that is keeping me from switching to nvim-surround at the moment.

kylechui commented 2 years ago

This sounds like a great feature that I would like to add, although I'm a bit busy with other things at the moment. I'll leave a few examples for myself below; feel free to correct if I've made any mistakes:

-- Before text
local f = function(arg1, arg2)
-- After `dsf`
local f = arg1, arg2
-- After `disf`
local f = function()
-- After `dasf`
local f = 

I think like you said, this shouldn't be too hard to implement; the main problem is how the user should configure this.

haoming-li-ling commented 2 years ago

I was playing around with the source, and put the following function in init.lua; I also bound das to call this function in config.lua by adding an entry for das in keymaps and calling set_keymap on it. However, it does the exact same thing as delete_surround; I am a little confused.

M.delete_object = function(args)
  -- Call the operatorfunc if it has not been called yet
  if not args then
    -- Clear the delete cache (since it was user-called)
    cache.delete = {}

    vim.go.operatorfunc = "v:lua.require'nvim-surround'.delete_callback"
    return "g@l"
  end
  local selection = utils.get_selection(args.del_char)

  -- Get the selections to delete

  if selection then
    buffer.delete_selection(selection)
    buffer.set_curpos(selection.first_pos)
  end

  buffer.reset_curpos(args.curpos)
  cache.set_callback("v:lua.require'nvim-surround'.delete_callback")
end

Am I missing something about what get_selection and delete_selection do?

haoming-li-ling commented 2 years ago

Oops, I closed this by mistake; I reopened; I hope you'll still be able to see this issue?

kylechui commented 2 years ago

It's all good, I can still see it. From a cursory glance it looks like your code should work; I would recommend re-trying the same edit on branch add-treesitter-support and see if that works?

Edit: Ah, the problem is that delete_callback will recall delete_surround, instead of delete_object. The control flow goes delete_surround (no args) → delete_callbackdelete_surround (with args). You can try writing another callback, but I probably won't merge it since I have a few other things related to callbacks that I want to test out.

haoming-li-ling commented 2 years ago

I wrote the appropriate callback and it worked. I will be happily using this local copy of the plugin for now. (das is all I need at the moment)

But I do hope to see the full functionality implemented officially someday!

kylechui commented 2 years ago

Glad that it works for you! If possible, I would like some suggestions on how this feature could be "properly" integrated. I think code-wise it shouldn't be that hard to implement; just have another function/callback pair, where the user is queried for the character. However, what would you recommend I call the keymaps? Perhaps delete_around and delete_inner to go with delete?

haoming-li-ling commented 2 years ago

Those seem to be good names, I think!

I guess there should be options to toggle these functionalities globally, and a note be added about overriding the builtin s "sentence" text object (useful for markdown and tex), which one may consider remapping.

Something more (maybe too) ambitious would be making the mappings available for all operators, not just deletion, so they become full-fledged text-objects (which is the case with vim-sandwich). Maybe this should be handled in a separate plugin, a Lua plugin to define user text-objects (which is still non-existent at present, I believe?).

kylechui commented 2 years ago

I think mini.ai is what you're looking for; I would prefer to not set any operator maps for this plugin; having it focus on just surrounds and not try to provide a whole framework for text-objects. On second thought, if there are conflicting keymaps, I'm unsure about whether or not I want to support this in nvim-surround. You're probably better off using something like nvim-treesitter-textobjects and just using daf.

haoming-li-ling commented 2 years ago

Oh, didn't know about mini.ai; I just tried it and it meets my needs (and saves one keystroke), only I don't know why it is slower than the modified nvim-surround when deleting simple regex patterns (there is a noticeable lag with mini.ai).

What I need to delete are special latex command sequences with text in between, so treesitter won't be of much help; only a regex-capable solution like mini.ai or nvim-surround will do.

kylechui commented 2 years ago

@Doltonius Does vim-sandwich literally create the operators asf/isf for you? Or does it only create the mappings dasf and disf, giving the illusion of custom text-objects? If I am to implement this, I think it might make more sense to do the latter, since for the former might introduce mappings that unintentionally conflict with other plugins, e.g. targets.vim, nvim-treesitter-textobjects, or even mini.ai. If the latter, then I figure that adding something similar to the local copy that you have already written would suffice. In any case this probably won't get added for a little bit (sorry!); I would like to see more support from the community for this feature before I go and just add it.

only I don't know why it is slower than the modified nvim-surround when deleting simple regex patterns

This is one of the most surprising things that I've heard so far, since I've done zero optimization and I already know of a few ways that I could speed up nvim-surround by quite a bit. However those performance issues probably will only show up on larger files with very long lines (if at all), which is why I haven't bothered trying to pre-optimize.

haoming-li-ling commented 2 years ago

vim-sandwich provides as<char> and is<char> as true textobjects that can be used after v, d, c, y and other operators, although this feature is disabled by default; the as<char> and is<char> mappings are only the suggested mappings. The conflict with the sentence text object is the only potential conflict I can think of with these mappings. Yeah, I agree that if there is only me who needs this, then probably it shouldn't be officially added; and even when added, I think the default should be to disable it as in vim-sandwich.

kylechui commented 2 years ago

It usually is quite hard to get a sense for how many people actually want this feature. I'll try to remember to make note of it in the next release post or something, but yeah unsure how many people are going from sandwich to this plugin, and from those how many used the text-objects.

I do think that disabling it by default makes sense though, leaving it for those that need it.

andrewferrier commented 2 years ago

So for what it's worth, I don't / haven't used this feature of vim-sandwich, but it does sound amazing, and I think I would try and make it part of my routine if it popped up in nvim-sandwich. To me, this is one of those features that "multiplies"; providing means you suddenly a whole bunch of useful text objects, all based on the built-in and user definitions for surroundings.

echasnovski commented 2 years ago

Oh, didn't know about mini.ai; I just tried it and it meets my needs (and saves one keystroke), only I don't know why it is slower than the modified nvim-surround when deleting simple regex patterns (there is a noticeable lag with mini.ai).

@Doltonius, 'mini.ai' really shouldn't have any noticeable lag when using Lua patterns: on my machine it almost always executes in less than 1 ms. The tree-sitter ones can be slow on big files (>1000 lines) with many captures: I sometimes get around 100 ms.

I would assume that issue here is with conflicting mappings (so one of them is applied after 'timeoutlen' ms) or something else. If you can reproduce the lag, would you mind opening an issue in 'mini.nvim'? Thanks.

ram02z commented 2 years ago

how many people are going from sandwich to this plugin, and from those how many used the text-objects

For what it's worth, I haven't migrated yet because this plugin doesn't expose the text-objects that vim-sandwich does

kylechui commented 2 years ago

Still delaying this feature request until later, but from a UX standpoint I think it might make sense to make it an additional (and optional) key into each surround, i.e. create_textobject = "some string". That way you could name them whatever you wanted, e.g. some people might prefer af instead of asf, or some other arbitrary string that isn't necessarily related to the input key into the surround (in the previous example f). This key would remain omitted by default, indicating that no text-object should be created.

kylechui commented 1 year ago

For those that are interested, someone else seems to have implemented this functionality as its own plugin: https://github.com/XXiaoA/ns-textobject.nvim

Note: nvim-surround is still required

XXiaoA commented 1 year ago

vim-sandwich has the functionality to turn user-define surrounds into textobjects through the recommended mappings as<char> and is<char>; suppose I have defined the char x for the delimiters foo and bar; then, for fooSOME_TEXTbar, dsx would result in SOME_TEXT, dasx would result in the entire phrase being deleted, and disx would result in foobar. Since we are already providing the find and delete patterns for user-defined surrounds, I think it would not be too difficult to implement this textobject functionality. This is the one vim-sandwich feature that is keeping me from switching to nvim-surround at the moment.

I'm confused about where will the cursor is after doing textobject. Maybe it isn't pretty important on delete operations, but it makes big effect on change operations.

kylechui commented 1 year ago

Ok so I don't think that implementing this feature would be terribly hard (famous last words), but I do have reservations for doing so since I feel that this would perhaps be beyond the scope of what nvim-surround is supposed to be used for. I was thinking of creating a "spin-off" plugin, i.e. nvim-motions, but the sheer amount of duplicated code that would have to be shared between the two seems almost comical. This feature does seem very useful and I would like to add it in; here are some ideas:

  1. Add it into nvim-surround as a create_motion key for each surround, which will use the find() key of the surround as the motion. a. Very good code reuse, no breaking changes, probably the easiest to actually do. b. Potentially "does too much" in one plugin, as we are now introducing motions to a plugin that is supposed to be focused on delimiter pairs.
  2. Create a new plugin nvim-motions. a. I would either need to duplicate a bunch of code (yuck), or have one of the plugins require the other (seems non-ideal as that would likely end up being a breaking change for nvim-surround).

Feel free to let me know what y'all think!

CCing the people who seem to want this: @Doltonius, @mawkler, @andrewferrier, @kuator, @ram02z, @wangweixuan

XXiaoA commented 1 year ago

I suppose we should create another plugin (do one thing and do it best). And maybe the new plugin had better be independent. In my view, It's annoying for user to require a surround plugin for a motion plugin (there seems has no relevance between then).

kylechui commented 1 year ago

Yeah I guess an alternate idea would be to introduce some sort of "library" that both plugins could inherit from, but I'm not 100% sure what that would look like. I think it might also be beneficial for other plugin developers, i.e. see this comment. I'm not sure if my code is something that can be put into LuaRocks, or how that works at all, but I'm open to any ideas.

XXiaoA commented 1 year ago

Maybe create a library which like https://github.com/nvim-lua/plenary.nvim provides find, buffer and stuff functionality🤔.

XXiaoA commented 1 year ago

This sounds like a great feature that I would like to add, although I'm a bit busy with other things at the moment. I'll leave a few examples for myself below; feel free to correct if I've made any mistakes:

-- Before text
local f = function(arg1, arg2)
-- After `dsf`
local f = arg1, arg2
-- After `disf`
local f = function()
-- After `dasf`
local f = 

I think like you said, this shouldn't be too hard to implement; the main problem is how the user should configure this.

Support now in ns-textobject with default configuration of nvim-surround and ns-textobject. (By using nvim-surround's surrounds and aliases configuration to create textobject automatically): a = func(args) Press dsf -> a = args Press daf -> a = Press dif -> a = func()

Hippo0o commented 1 year ago

utilizing mini.ai i can define it like this:

  local ns_utils = require("nvim-surround.utils")

  local function ns_alias_mini_ai(alias)
      return function (mode)
        local s = ns_utils.get_nearest_selections(alias, "change")
        if not s then
          return
        end
        local f_col = s.left.first_pos[2] + (mode == "i" and 1 or 0)
        local t_col = s.right.last_pos[2] + (mode == "i" and -1 or 0)
        local region = {
          from = { line = s.left.first_pos[1], col = f_col },
          to = { line = s.right.last_pos[1], col = t_col },
        }
        return region
      end
  end

  local custom_textobjects = {}
  for alias, _ in pairs(require('nvim-surround.config').get_opts().aliases) do
    custom_textobjects[alias] = ns_alias_mini_ai(alias)
  end

  require("mini.ai").setup({
    custom_textobjects = custom_textobjects,
    silent = true,
  })

i think mini intercepts ns_utils.get_nearest_selections so it has forward search

kylechui commented 3 months ago

After further consideration (a year later :skull:) I think this is probably beyond the scope of this particular plugin; alternatives linked earlier in this thread should suffice for most people.

If anything, I think the reverse approach is correct: first build a text-object/motion using something like mini.ai, then use that motion in nvim-surround to operate on it.