Tree-sitter user interface

[kylechui]

With the upcoming Tree-sitter support, one of the problems is that each language uses a different naming scheme for defining what a "function call" is. For example:

• Lua: function_call
• Python: call
• C++: call_expression

Since I want to include some Tree-sitter configuration by default, there needs to be a way for the user to "pick and choose" which filetypes they want Tree-sitter configurations for. As suggested by emi2k01, I am currently using lspconfig-style configuration, e.g.

-- Uses the Tree-sitter node `function_call` instead of pattern-matching for function calls
require("nvim-surround.filetype").lua.setup()

Are there any alternative ways to setup these "filetype-specific" defaults that anybody has that might be more intuitive? With the current configuration I fear that users might put their filetype-specific configurations into the setup function, expecting it to be called via an autocommand. If you think that I'm overthinking this and the current situation is fine, feel free to leave a 👍 or a comment below. Thanks!

[andrewferrier]

So I'm not quite clear what you/emi2k01 are suggesting/currently doing?

If all the filetype-specific configuration is doing is tracking what a function call is called in Treesitter for that lang, is it necessary for the user to explicitly setup() each filetype? Can you not just automatically load all the configuration? It will be tiny. See what I do here with filetypes.lua as a similar concept.

The reason lspconfig does it that way, I think, is because in setting those up, you are actually configuring autocmds to listen for particular filetypes, start language servers (which may or may not be installed on a per-filetype basis), etc. I think your needs are much simpler, aren't they?

Treesitter itself does not make you specify each language you want to use (at least as long as you accept the default of all)!

[kylechui]

Hmm... upon literally 10 seconds of further thinking, perhaps we can expose the autogroup to the end user, and they can delete/clear it if they don't like it? I guess at this point I'm thinking about the users that want to squeeze every last millisecond out of their startup time.

[smjonas]

and they can delete/clear it if they don't like it

Tbh, I don't think that will be necessary, as Andrew said there's not really much going on in this autocommand anyway. Performance-wise, there probably won't even be a measurable difference. If you mean entirely disabling treesitter surrounds however, that could be an option of course :)

Initially, splitting the filetype-specific Treesitter configuration into separate files / tables seemed like a good idea to me. The main reason for that was because I wasn't sure how much complexity there will be for each filetype. But now, I would keep them in a single file (as you are doing now) and simply call buffer_setup which is called from a single autocommand as opposed to one autocommand per filetype. I.e. I would remove the require("nvim_surround.filetype").setup() functionality entirely. As a benefit, that reduces the amount of complexity / required configuration that's passed on to the user.

[kylechui]

Ok, I just have everything set up by default now on the latest commit for v2.0.0; please test it whenever y'all have the time.

Note to self: It should probably just do nothing if the Tree-sitter language server isn't set up for the given filetype, and use the defaults. Still haven't gotten around to looking at this comment.

[kylechui]

@smjonas @andrewferrier New questions:

• How can/should the user disable these filetype-specific mappings?
  • I was thinking perhaps a filetype_surrounds = true/false in the setup?
  • I'm also thinking that maybe wanting Tree-sitter surrounds for filetype A but not B is going to be a relatively niche case, and in that case they can copy-paste from filetype.lua into ftplugin/[lang].lua or something
• What should nvim-surround do in the case that there doesn't exist a Tree-sitter parser for the filetype? NOTE: This has been implemented in the latest commit on v2.0.0, to fallback on default config (e.g. for function calls, use Lua patterns)
  • I think that issuing a warning time and time again would get annoying, especially if the parser is intentionally left uninstalled (e.g. if the Tree-sitter parser is broken/not great)
  • My guess is that it should directly default to whatever is stored in config.lua

[kylechui]

I have no clue how to include filetype-specific maps without absolutely trashing all user-defined filetype-specific maps. Ideally the hierarchy would go:

1. Global default config gets loaded
2. Global user config gets merged in (forcefully)
3. Buffer-local default config gets merged in (forcefully)
4. Buffer-local user config gets merged in (forcefully)

The unfortunate thing here is that in steps 3 and 4, the execution order is actually reversed, since ftplugin gets executed before autocommands do, which causes incorrect behavior. The last resort idea that I have in my head is to assign every single surround a "priority" that's invisible to the user, and set priorities "behind the scenes" for user/default configs, but IMO it's quite ugly.

[kylechui]

Separate idea: since we already have things like f for function call built-in, and are going to be loading in some "opinionated" defaults, I wonder if it makes sense to also define some more mappings for other languages, e.g. c and e for command and environment in LaTeX. The main issue at hand is ideally all of the "default" autocommands would be run before any user autocommands, so users could pick and choose disabling defaults, just like they currently do with the global config. However I don't really know of any way to "ensure" nvim-surround autocommands get executed before user ones, or before ftplugin functions. Curious to know what everybody's thought are on "how far" the defaults should go

[kylechui]

One thought I have right now is to add a new key to setup (invisible to the user) that tells nvim-surround if the given setup is a default or set by the user, then "hard"/"soft" merge the configs that way. Hopefully that would make configurations commute and make filetype-specific configs behave identically to global configs.

[kylechui]

This idea has many issues involving synchronizing behavior and ensuring that all the merges occur in the right order etc., probably not worth it.

[kylechui]

Ok even ignoring all of the previous statements, there's a need for an interface for getting the pair of selections from the buffer, Tree-sitter style. This is because for the following cases, there is no Lua pattern that can properly handle deleting the function call:

print(":)")
print(":(")

[kylechui]

Nevermind, it's just Lua in particular that has weird edge cases like

require("nvim-surround").setup({ move_cursor = false })

which has multiple sets of parentheses in the match (since the entire line is technically a function_call node). I think I will... simply ignore this for now.

[kylechui]

As it stands right now, the Lua pattern-based implementation for changing function calls only replaces the "final part" of the function call, e.g. the replaced parts are underlined here:

func_call("Hello world")
.........
require("nvim-surround").normal_surround()
                         ...............
ClassName->method(args)
           ......

I wonder if there's a way to actually implement this using queries? It seems like I would have to write/maintain all the queries myself, and I'm not 100% sure if that's feasible/reasonable. I also am unsure of whether or not this can use nvim-treesitter-textobjects, but it seems like the answer is no. Sorry for the mention @clason, but I feel like you might have a better understanding of how this could be done?

[kylechui]

The original plan I had was to just pattern-match the @call.outer query with .-%(.-$), but that seems to fail in the middle case above (or any case where there are multiple function calls as a part of the overarching function call.

[clason]

In principle, this should be something like

(function_declarator
  declarator: (identifier) @function)
(function
  declarator: (identifier) @function)

but I don't lisp, and this is completely up to the parser.

But, yes, in general you are supposed to write specific queries for such a purpose (of course, you can steal adapt from others, like textobjects).

[beardedsakimonkey]

Hi, with a634889cb4a02b370f5c5e51c925ef1bc8b1982f, it seems like the issue has been remedied a bit by enabling use of query captures, which can come from nvim-treesitter-textobject. Given that, I'm wondering if we still intend to include file type-specific treesitter definitions.

[kylechui]

I no longer intend on introducing default filetype-specific definitions; however, you may still define them yourself. To put it mildly, trying to set up the priority hierarchy between default/custom global/buffer-local definitions was awful. Tree-sitter textobjects are "the way to go" for most things, and any residual gaps should be filled in via user configuration, or upstream fixes to Tree-sitter textobjects.

[clason]

I notice that you unconditionally require'nvim-treesitter' -- I don't think you should do that, as that plugin may not be installed (or not yet loaded).

You should at least do something like

local has_ts, _ = pcall(require, "nvim-treesitter")
if has_ts then
 ...
end

to avoid trying to require the heavy submodules query and ts_utils (which itself pulls in more submodules).

[kylechui]

Is this for the config file? I thought I had included a safeguard against that by first checking if Tree-sitter was loaded in (via vim.g.loaded_nvim_treesitter)

[clason]

I don't think the requires are guarded? But I might have missed something in the control flow.

[kylechui]

I think I'll update the code to have all the checks inside the actual files themselves (i.e. in queries.lua and treesitter.lua), and the functions will just return nil if it's not installed there.