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

Record variable declaration ranges #880

Merged
merged 4 commits into from
May 11, 2021
Merged

Conversation

turbolent
Copy link
Member

Description

Record the ranges in which variable declarations are available, if position information is enabled (off by default).

This feature can be used to provide completion items in the language server.

Also, remove the unused Hash function for sema.Position and the dependency on fasthash.


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@turbolent turbolent self-assigned this May 8, 2021
Copy link
Contributor

@janezpodhostnik janezpodhostnik left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -40,12 +40,13 @@ func (position Position) Shifted(length int) Position {
}
}

func (position Position) Hash() (result uint32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this not used?

Copy link
Member Author

Choose a reason for hiding this comment

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

It had been previously used / was previously needed when we used a persistent map/set library that used this function for the key. We replaced it and removed the dependency a while ago, and this was a left-over

Copy link
Member

@SupunS SupunS 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!

@turbolent turbolent merged commit bee1090 into master May 11, 2021
@turbolent turbolent deleted the bastian/declaration-ranges branch May 11, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants