Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
telemachus opened this issue Sep 5, 2021 · 21 comments
Open

Comments

@telemachus
Copy link
Contributor

(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
Copy link
Member

stsewd commented Sep 10, 2021

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
Copy link
Member

stsewd commented Sep 10, 2021

Looks like we have the same problem with C :/

@mawkler
Copy link
Contributor

mawkler commented Oct 29, 2021

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
Copy link

dominikh commented Dec 17, 2021

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
Copy link
Member

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
Copy link
Contributor

mawkler commented Dec 20, 2021

@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
Copy link

@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
Copy link
Contributor

mawkler commented Dec 21, 2021

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
Copy link
Member

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
Copy link

arsham commented Jan 19, 2022

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

telemachus commented Apr 5, 2022

@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
Copy link
Member

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

@telemachus
Copy link
Contributor Author

telemachus commented Apr 5, 2022

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

@jjant
Copy link

jjant commented Oct 10, 2022

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

@telemachus
Copy link
Contributor Author

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
Copy link

jjant commented Oct 10, 2022

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
Copy link
Contributor

icyd commented Oct 17, 2022

This PR #314 should solve this issue

@Goxore
Copy link

Goxore commented Nov 8, 2022

I have the same issue with c#

@icyd
Copy link
Contributor

icyd commented Nov 8, 2022

I have the same issue with c#

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

@ilan-schemoul
Copy link
Contributor

ilan-schemoul commented Apr 2, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants