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

Autocomplete: upgrade tree-sitter and expand language support #3373

Merged
merged 3 commits into from
Mar 11, 2024

Conversation

valerybugakov
Copy link
Member

@valerybugakov valerybugakov commented Mar 11, 2024

Context

  • Upgrade tree-sitter from 0.20.8 to 0.21.0 and update the code based on breaking changes.
  • Migrate from custom S3-hosted grammars to the latest grammars from npm via tree-sitter-wasms. This expands the tree-sitter language support and improves grammars error tolerance.
  • Add timeout to the tree-sitter parse method to avoid freezing users' editor instances in case of unexpected errors.
  • Refactor how tree-sitter edits are calculated to paste completions into the existing document. This fixes the issue described in the Slack thread.
  • Updated the SupportedLanguage enum to keep all keys and values equal to the possible document.languageId values.

Test plan

  • I could not repro the infinite loop parsing issue in our integration tests environment.
  • CI and manual tests from the Slack thread.

Comment on lines +67 to +69
// stop parsing after 50ms to avoid infinite loops
// if that happens, tree-sitter throws an error so we can catch and address it
parser.setTimeoutMicros(50_000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we log the error anywhere? It'd be useful to know how often this times out in prod

Copy link
Member Author

Choose a reason for hiding this comment

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

Our Sentry setup should pick it up on prod.

Comment on lines +158 to +162
(interface_body
(property_signature
name: (property_identifier) @signature.property)))
(interface_declaration
(object_type
(interface_body
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for upgrading! I noticed this difference when trying to copy a query from the playground, definitely something we'll have to be proactive about but great that we have tests for these queries :)

I've been adding new queries in #3275 so I'll rebase that onto this

vscode/src/tree-sitter/grammars.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@philipp-spiess philipp-spiess left a comment

Choose a reason for hiding this comment

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

Awesome!

Co-authored-by: Tom Ross <tom@umpox.com>
@valerybugakov valerybugakov enabled auto-merge (squash) March 11, 2024 12:09
@valerybugakov valerybugakov merged commit 39b90bf into main Mar 11, 2024
16 checks passed
@valerybugakov valerybugakov deleted the vb/tree-sitter-edits branch March 11, 2024 12:13
steveyegge pushed a commit that referenced this pull request Mar 13, 2024
- Upgrade tree-sitter from `0.20.8` to `0.21.0` and update the code based on breaking changes.
- Migrate from custom S3-hosted grammars to the latest grammars from `npm` via [tree-sitter-wasms](https://github.com/Gregoor/tree-sitter-wasms). This expands the tree-sitter language support and improves grammars error tolerance.
    - Follow-up issue #3374
- Add timeout to the tree-sitter parse method to avoid freezing users' editor instances in case of unexpected errors.
- Refactor how tree-sitter edits are calculated to paste completions into the existing document. This fixes the issue described in [the Slack thread](https://sourcegraph.slack.com/archives/C05AGQYD528/p1709892644622649?thread_ts=1709891165.036869&cid=C05AGQYD528).
- Updated the `SupportedLanguage` enum to keep all keys and values equal to the possible `document.languageId` values.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants