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

Add tree-sitter based function, class navigation #1619

Merged
merged 3 commits into from
Feb 15, 2022

Conversation

sudormrfbin
Copy link
Member

Adds support for navigating between functions, classes, etc, using tree-sitter and textobjects. Requires that a *.around capture is defined for a filetype in it's correspoding textobjects.scm file. So for goto next function to work in Rust, function.around textobject has to be defined in Rust's textobjects.scm.

helix-tree-sitter-navigation

Added keybinds (click to expand)
Keybind Description
]f Go to next function
[f Go to previous function
]c Go to next class
[c Go to previous class
]p Go to next parameter
[p Go to previous parameter

Open questions

  • Should we fall back to using the inside textobject if around is not defined ? Currently parameter.around is not defined in any query file, so ]p is not very useful.

@EpocSquadron
Copy link
Contributor

I was thinking parameter.around, if it were added should either select the full parameter list, or the parameter plus the delimiter (so fn([one, ]two), where the square brackets represent the selection). Either way I think it would be more useful for this keybind to use .inside for parameters specifically.

@EpocSquadron
Copy link
Contributor

EpocSquadron commented Feb 3, 2022

How would we handle for comment navigation when I add that? Should that use around, or be more granular?

What if we gave the user a choice by doing something like:

]f - next function body (inside)
]F - next complete function (around)

@sudormrfbin
Copy link
Member Author

the parameter plus the delimiter (so fn([one, ]two), where the square brackets represent the selection

This was what I initially wanted to do, but I ran into some problems. We would probably need something similar to neovim's custom #make-range! predicate to handle this properly.

How would we handle for comment navigation when I add that? Should that use around, or be more granular?

I think around would work fine for it too, since then we can navigate by comment blocks, which seems intuitive. If we use inside for comment navigation each single line comment would be a separate node and there j and k would supersede it.

What if we gave the user a choice by doing something like:

]f - next function body (inside)
]F - next complete function (around)

That IMO adds a bit too much verbosity that doesn't seem very useful since you can always use mif and maf after getting to the function.

@EpocSquadron
Copy link
Contributor

@sudormrfbin ok, agreed on all counts. What about introducing a new subcapture meant specifically for this? Something like @function.movement that could be used alongside to get around having to fall back to .inside eg:

(anonymous_function_creation_expression
  body: (_) @function.inside) @function.around @function.movement
  
(formal_parameters
  [
    (simple_parameter)
    (variadic_parameter)
    (property_promotion_parameter)
  ] @parameter.inside @parameter.movement)

I just tested this out in PHP with tree-sitter-cli and it did indeed allow multiple captures on the same nodes (so @function.around and @function.movement were both captured and both corresponded to the same items):

  pattern: 8
    capture: 5 - parameter.inside, start: (158, 37), end: (158, 51), text: `$block = false`
    capture: 6 - parameter.movement, start: (158, 37), end: (158, 51), text: `$block = false`
  pattern: 5
    capture: function.around, start: (158, 4), end: (167, 5)
    capture: function.movement, start: (158, 4), end: (167, 5)
    capture: function.inside, start: (159, 4), end: (167, 5)

@EpocSquadron
Copy link
Contributor

EpocSquadron commented Feb 3, 2022

I'm also having trouble getting the parameter movements to work in PHP and Rust files after checking out this PR and trying it out. Functions and classes work beautifully.

Edit: of course it doesn't work we haven't implemented the parameter thing yet! Ignore my comment.

@sudormrfbin
Copy link
Member Author

Added an object.movement capture that can be used to override object.around and object.inside, and the precedence order is now movement, around, inside. I haven't added the movement capture to any file yet (with regard to parameter captures), since it works fine for now.

See the [unimpaired][unimpaired-keybinds] section of the keybind
documentation for the full reference.

> NOTE: This feature is dependant on tree-sitter based textobjects
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> NOTE: This feature is dependant on tree-sitter based textobjects
> NOTE: This feature is dependent on tree-sitter based textobjects

some grammars](https://github.com/search?q=repo%3Ahelix-editor%2Fhelix+filename%3Atextobjects.scm&type=Code&ref=advsearch&l=&l=)
currently have the query file implemented. Contributions are welcome !
some grammars][lang-support] currently have the query file implemented.
Contributions are welcome !
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Contributions are welcome !
Contributions are welcome!

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just needs a rebase 👍🏻

@archseer archseer merged commit ab2a0f3 into helix-editor:master Feb 15, 2022
@sudormrfbin sudormrfbin deleted the tree-sitter-navigation branch February 15, 2022 05:06
EpocSquadron added a commit to EpocSquadron/helix that referenced this pull request Feb 20, 2022
EpocSquadron added a commit to EpocSquadron/helix that referenced this pull request Mar 1, 2022
EpocSquadron added a commit to EpocSquadron/helix that referenced this pull request Mar 4, 2022
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

Successfully merging this pull request may close these issues.

3 participants