kylechui / nvim-surround

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

feat: Native Tree-sitter support. #123

Closed kylechui closed 2 years ago

kylechui commented 2 years ago

Add native Tree-sitter support in the config.get_selection() function, as a way of finding the "nearest" Tree-sitter node of that type. As of right now users can utilize pattern or textobject to get the "parent" selections, but they are subject to inaccuracies, e.g. dsf on

func_name(arg1, ")")

To remedy this, it would be better to be able to have something like this:

local config = require("nvim-surround.config")
require("nvim-surround").setup({
    ["f"] = {
        -- add, delete, change keys here
        find = function()
            return config.get_selection({ node = "function_call" })
        end
    }
})

Also I'm still not sure what I should name the new API, e.g. node or ts_node or treesitter_node. I'm currently leaning towards node since it seems more similar to textobject and pattern, e.g. I didn't name them vim_textobject and lua_pattern. In any case, I would like to hear any thoughts people might have about this feature :)

Development for this feature will occur on add-treesitter-support

kylechui commented 2 years ago

First version is up and running; it uses Tree-sitter by default for finding function calls, and falls back on Lua pattern-matching if no node is found. Note that different parsers call these different names, e.g. in Lua they are called function_call and in Python it is called call. This can be overridden with buffer-local configuration, where the entire key is copy-pasted from the default and the differing node is provided:

local config = require("nvim-surround.config")
require("nvim-surround").buffer_setup({
    surrounds = {
        ["f"] = {
            add = function()
                local result = config.get_input("Enter the function name: ")
                if result then
                    return { { result .. "(" }, { ")" } }
                end
            end,
            find = function()
                return config.get_selection({ node = "call" }) or config.get_selection({ pattern = "[%w%-_:.>]+%b()" })
            end,
            delete = "^([%w%-_:.>]+%()().-(%))()$",
            change = {
                target = "^.-([%w_]+)()%b()()()$",
                replacement = function()
                    local result = config.get_input("Enter the function name: ")
                    if result then
                        return { { result }, { "" } }
                    end
                end,
            },
        },
    },
})

This allows for better matching against situations like the following:

-- Before
func_call("this has a ) closing paren")
-- Command: dsf
"this has a ) closing paren"

In the old versions, the Lua pattern matching would match the first closing parenthesis, deleting the wrong delimiter pair.

Edit: Mini-list of these names:

A part of me wants to set this up for the user by default, but I'm not sure how to do so in a way that makes sense, given the current configuration options.

smjonas commented 2 years ago

Great improvement, node sounds good to me!

kylechui commented 2 years ago

A part of me wants to set this up for the user by default, but I'm not sure how to do so in a way that makes sense, given the current configuration options.

I wonder if I should create a new "mini-plugin" that's just a list of configuration options for Tree-sitter nodes, and have the default be "dumb". As a side note, does anybody know how to check if a Tree-sitter server language parser thing is installed for the current buffer/filetype?

smjonas commented 2 years ago

I don't think a separate plugin would be useful here. I feel like that would just confuse more users (e.g. why is another plugin needed?). Though you could of course put that in a separate module or make it opt-in.

As a side note, does anybody know how to check if a Tree-sitter server language parser thing is installed for the current buffer/filetype?

Yes, here's how nvim-treesitter does it: https://github.com/nvim-treesitter/nvim-treesitter/blob/a5eafa3b6d64ae2706d68e4ddde8e79eada0ff03/lua/nvim-treesitter/install.lua#L89 (you need to convert the buftype to a parser language first, see here: https://github.com/nvim-treesitter/nvim-treesitter/blob/master/lua/nvim-treesitter/install.lua#L432 for an example)

kylechui commented 2 years ago

I don't think a separate plugin would be useful here. I feel like that would just confuse more users (e.g. why is another plugin needed?). Though you could of course put that in a separate module or make it opt-in.

This is reasonable; I guess the main thing I'm concerned about is creating autocommands/maintaining a list of Tree-sitter node types for associated languages, all in the default configuration. It just seems like it could get bloated reasonably quickly, especially if everybody wants "their particular language" to be supported. Either a separate plugin that's just full of buffer-local configurations or maybe a wiki page seems like it would work, but again I feel like it's going to end up being a non-trivial amount of code that would clutter user configuration.

smjonas commented 2 years ago

I guess the main thing I'm concerned about is creating autocommands/maintaining a list of Tree-sitter node types for associated languages, all in the default configuration

By "default configuration" do you mean the current config.lua file? I would definitely try to keep the treesitter stuff in separate modules. The treesitter surrounds could then be optionally required from config.lua and merged with the default surrounds. But I generally think it would be very valuable to include language-specific treesitter configuration in the plugin itself.

kylechui commented 2 years ago

Hmm perhaps a sub-directory full of language-specific modules would be a good place to start; please let me know if you have any specific ideas on how this could be implemented! I was thinking something like require("nvim-surround.setup.lua") or something like that, where the associated module is just a buffer_setup call inside an autocommand.

smjonas commented 2 years ago

That sounds like a good first start! A separate file for each supported language (as well as maybe a base module with common utility functions) would be what I had imagined as well. By the way, will there be textobjects other than for functions similar to treesitter-text-objects? Or is that something to consider for later versions?

kylechui commented 2 years ago

Not entirely sure what that last statement means. The textobject key for the get_selection function uses any motion that's set, e.g. af if the f motion is defined by nvim-treesitter-textobjects. The idea here for this branch is that you can directly use Tree-sitter nodes, so "polluting" your operator maps isn't necessary anymore.

Edit: If you're referring to adding more "default" maps, I think that's up to "popular demand". Using f for function calls seemed especially popular, which is why it was included as a default.

kylechui commented 2 years ago

I wonder what sort of interface should be used to load in all of the language-specific modules. So far I've set it up to be require("nvim-surround.languages.lua"), but the name seems kinda off to me. Other recommendations would be appreciated.

kylechui commented 2 years ago

@andrewferrier @NoahTheDuke @smjonas On second thought, I think I'll try the naming scheme:

require("nvim-surround.languages").setup("lua")

So the user can just use the same function all the time, passing in their language of choice for Treesitter support.

emi2k01 commented 2 years ago

I'm not familiar enough with Lua to know how a good api would look on it but the api followed by lspconfig seems good.

require("nvim-surround.languages").lua.setup()

kylechui commented 2 years ago

This has been updated in the most recent commit; the file languages has also been renamed to filetype to be more suggestive as to what the keys should be. For example, typescript files use the typescriptreact key instead of tsx or typescript. Short sample config:

require("nvim-surround").setup()
require("nvim-surround.filetype").lua.setup()
require("nvim-surround.filetype").typescriptreact.setup()

@emi2k01 You can try using the typescript setup function and see how it handles your particular use cases. This file is probably going to be largely community-driven, so feel free to make any suggestions for any languages you would like to see supported!

kylechui commented 2 years ago

I feel like this is more or less done? It seems to work just fine on my end for Lua, as well as when I was playing around with it for TypeScript. By this I mean the mechanism should be more or less complete, but the default configurations in filetype.lua are not, which I can always get more help with :). If anybody's interested in opening a PR to the v2.0.0 branch or just tagging me in an issue here, they can help contribute by noting down what the function call node type is for a given Tree-sitter parser (I used the playground for this).

kylechui commented 2 years ago

See #137 for further discussion on user interface.