nvim-treesitter / nvim-treesitter-textobjects

Apache License 2.0
2.18k stars 195 forks source link

Question: is it expected that inner function objects include braces? #112

Open telemachus opened 3 years ago

telemachus commented 3 years ago

(Thanks for the plugin: it is a snap to set up and easy to work with so far.)

I was surprised to see that by default, an inner function object in, for example, Go and Javascript includes the braces. I'm guessing that this is expected behavior and probably a result of how treesitter itself works. But I wanted to confirm that before going further.

I ask because my most common use for inner function objects is that I want to change, delete, or yank all the code in the function body, but in those cases I don't normally want to include the braces. So, I'll need to consider whether it's possible to tweak treesitter's results.

stsewd commented 3 years ago

They shouldn't include braces. Depending on the grammar this could be easy to fix or not... For js and go, looks like it require for us to support the */+ operators.

(function_declaration
  body: (statement_block . "{" (_)* @function.inner . "}")) @function.outer

Could also be fixed by the grammar to not include braces inside the block node.

stsewd commented 3 years ago

Looks like we have the same problem with C :/

mawkler commented 3 years ago

I've also noticed this and want to add that i think it would be great if @function.inner left out potential line breaks, so that in the following example:

function foo() {
  let bar = 42;
  return bar;
}

pressing dif yields this:

function foo() {
}

This means that if should be linewise, just like ip.

However, I think that there should be an exception for cif, where an empty line is left inside the function for you to start typing inside it.

function foo() {
  let bar = 42;
  return bar;
}

pressing cif yields this (with the cursor shown as |):

function foo() {
  |
}

This is similar to how d in visual-line mode removes all selected lines, but c in the same mode leaves one line for you to type on.

Similarly I've noticed that af doesn't include trailing newlines after the function like ap does for paragraphs. I think it would make sense if af behaved as similar to ap as possible. That means that it should behave linewise in visual mode as well. What's great a behaviour like this is that one for example could use daf to move a function to a different place in a file and simply press p on an empty line and all newlines would be included in a correct way.

Note also that if there is is no newline after the function, for instance if it's at the end of the file the newline above the function should be selected instead, which is how ap behaves.

dominikh commented 2 years ago

I don't think that linewise makes sense for af, since functions can be expressions in many languages, like in the following snippet of javascript:

var x = function() { 
  console.log("Hello, world")
}

I do agree that af (and other as) should include whitespace, however.

theHamsta commented 2 years ago

It is a short-coming that we can only select nodes at the moment. We wanted to trim the braces using a regex directive and use that solution also for language injection. Language injection now uses a hack #offset to trim characters. Would be great when someone could contribute a proper solution!

mawkler commented 2 years ago

@dominikh

I don't think that linewise makes sense for ap, since functions can be expressions in many languages

I'm not sure that I'm following, could you please elaborate? Also, did you mean to write ap, or should it be if?

dominikh commented 2 years ago

@Melkster I meant to write af, not ap. And my point was that if af acted linewise, then doing something like daf inside the function in

var x = function() { 
  console.log("Hello, world")
}

would delete more than just the function.

mawkler commented 2 years ago

Perhaps there should be an additional command for a linewise "around function" text-object, for instance Af, kinda like how targets.vim allows an upper-case "around" to do things like A(, A{, which includes any trailing whitespace.

I personally think that it would be a super useful feature to be able to move around functions using a single text-object which gets the whitespace right immediately, just like one can do with paragraphs and ap.

theHamsta commented 2 years ago

daf should really only delete the actual function. To delete line-wise you can use dVaf. Maybe one could have an option for users to make certain mappings automaticly line-wise.

arsham commented 2 years ago

I'm trying to exclude the braces for go based on the above suggestion, the it didn't match the inner function (or block of code actually). I've tried these variations:

(function_declaration
  body: (block . "{" (_)* @function.inner . "}")) @function.outer

And

(func_literal
  body: (block . "{" (_)* @function.inner . "}")) @function.outer

I have the latest Neovim and all related plugins in my setup.

Thank you.

telemachus commented 2 years ago

@arsham I haven't found a solution yet with this repository, but you may find nvim-treesitter-textsubjects helpful. (It only works for a handful of languages so far, but it's inner object works well with go.)

telemachus commented 2 years ago

@stsewd @theHamsta until * and + are supported for captures, would you consider a patch that uses the pattern from nvim-treesitter-textsubjects for inner functions? It definitely seems like the consensus that an inner function text object should not include the braces. The following work well for go, for example.

(function_declaration
  (block . "{" . (_) @_start @_end (_)? @_end . "}"
         (#make-range! "myfunction.inner" @_start @_end)))

(method_declaration
  (block . "{" . (_) @_start @_end (_)? @_end . "}"
         (#make-range! "myfunction.inner" @_start @_end)))

(func_literal
  (block . "{" . (_) @_start @_end (_)? @_end . "}"
         (#make-range! "myfunction.inner" @_start @_end)))
theHamsta commented 2 years ago

Would be ok. If possible all "{" should be patched

telemachus commented 2 years ago

If possible all "{" should be patched

Yup, understood. I'm not sure how many of those languages I would immediately recognize as such, but I can do several, start a PR, and then get feedback about others to fill in. I'm happy to do that if it works for people.

Update: I've got the following languages drafted. If anyone can think of an obvious brace language that I've missed, please let me know. My plan is to clean these up and submit an initial PR this weekend.

jjant commented 2 years ago

This is still broken/wrong for @function.inner in Rust, and @block.inner. Both include the curly braces.

telemachus commented 2 years ago

This is still broken/wrong for @function.inner in Rust, and @block.inner. Both include the curly braces.

Yup. Unfortunately as I mentioned soon after, I didn't think to include Rust. I don't use the language, and I didn't realize that it was affected when I first made the pull request.

There are other ways in which my initial pull request probably needs to be expanded. I meant to do that myself, but I frankly forgot.

I definitely won't get to this quickly (I'm a teacher, and it's recommendation-writing season), but by looking at the changes I made to other languages, someone else could probably fix Rust pretty quickly. (You can also test it locally on your machine if you follow the instructions to override predefined textobjects.)

jjant commented 2 years ago

Ah, I see, I missed your comment! Thanks for the pointers, I'll give it a try myself (if/when I've got the time 😅).

icyd commented 2 years ago

This PR #314 should solve this issue

Goxore commented 2 years ago

I have the same issue with c#

icyd commented 2 years ago

I have the same issue with c#

I don't use C#, please verify this work for you #318

ilan-schemoul commented 7 months ago

I do not know if it's related but in c/cpp @block.outer includes braces (and @block.inner selects random stuff) #589