nvim-treesitter-textobjects icon indicating copy to clipboard operation
nvim-treesitter-textobjects copied to clipboard

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

Open telemachus opened this issue 2 years ago • 21 comments

(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.

telemachus avatar Sep 05 '21 14:09 telemachus

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 avatar Sep 10 '21 17:09 stsewd

Looks like we have the same problem with C :/

stsewd avatar Sep 10 '21 17:09 stsewd

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.

mawkler avatar Oct 29 '21 16:10 mawkler

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.

dominikh avatar Dec 17 '21 16:12 dominikh

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!

theHamsta avatar Dec 17 '21 17:12 theHamsta

@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?

mawkler avatar Dec 20 '21 21:12 mawkler

@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.

dominikh avatar Dec 20 '21 22:12 dominikh

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.

mawkler avatar Dec 21 '21 09:12 mawkler

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.

theHamsta avatar Dec 21 '21 13:12 theHamsta

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.

arsham avatar Jan 19 '22 05:01 arsham

@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 avatar Mar 28 '22 18:03 telemachus

@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)))

telemachus avatar Apr 05 '22 17:04 telemachus

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

theHamsta avatar Apr 05 '22 19:04 theHamsta

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.

  • bash
  • c
  • c++ (no changes to the c++ queries file, but handled because c++ relies on c queries)
  • cuda (cuda relies on c++)
  • dart
  • go
  • java
  • javascript
  • php

telemachus avatar Apr 05 '22 19:04 telemachus

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

jjant avatar Oct 10 '22 14:10 jjant

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.)

telemachus avatar Oct 10 '22 14:10 telemachus

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 😅).

jjant avatar Oct 10 '22 14:10 jjant

This PR #314 should solve this issue

icyd avatar Oct 17 '22 22:10 icyd

I have the same issue with c#

Goxore avatar Nov 08 '22 11:11 Goxore

I have the same issue with c#

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

icyd avatar Nov 08 '22 19:11 icyd

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

ilan-schemoul avatar Apr 02 '24 00:04 ilan-schemoul