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

LSP: Refactor definition to be similar to hover feature #595

Merged
merged 1 commit into from
May 6, 2023

Conversation

jansul
Copy link
Contributor

@jansul jansul commented May 5, 2023

Unfortunately it's a little hard to see the actual change in this PR between all the red and green, so hopefully some description helps.

After understanding how the hover implementation works a little better, I've refactored the definition code to be more in line with it.


The biggest change is that I have reverted which nodes we track (see track: false everywhere), so we are no longer tracking more specific nodes such as component names etc

Instead, there is a new method cursor_intersects? which can be used to check if a node is currently being hovered over. We can use this to exit early when trying to find the definition for a "higher level" node.

e.g.

def definition(node : Ast::HtmlComponent, server : Server, workspace : Workspace, stack : Array(Ast::Node))
  
  # We are not hovering over the component name, so don't bother looking for the definition
  return unless cursor_intersects?(node.component)

  # ...
end

This removes the need for StackReader, as that was mostly used to find the reverse ("higher level" nodes from more specific ones)


I've implemented some more of the easier "Go to Definitions" (for a separate PR for ease of review!) and it doesn't seem any more difficult to implement using this method.

Copy link
Member

@Sija Sija left a comment

Choose a reason for hiding this comment

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

Looks awesome 🚀

src/ls/definition.cr Show resolved Hide resolved
src/ls/definition/html_element.cr Outdated Show resolved Hide resolved
@Sija Sija merged commit 2296ea5 into mint-lang:master May 6, 2023
@Sija Sija added this to the 0.18.0 milestone May 6, 2023
@Sija Sija added tooling Tooling related feature (formatter, documentation, production builder) refactor labels May 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor tooling Tooling related feature (formatter, documentation, production builder)
Development

Successfully merging this pull request may close these issues.

3 participants