Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

WIP: spiked tree-sitter-dart impl#57987

Closed
matthewnitschke-wk wants to merge 6 commits into
sourcegraph:mainfrom
matthewnitschke-wk:tree_sitter_for_dart
Closed

WIP: spiked tree-sitter-dart impl#57987
matthewnitschke-wk wants to merge 6 commits into
sourcegraph:mainfrom
matthewnitschke-wk:tree_sitter_for_dart

Conversation

@matthewnitschke-wk

Copy link
Copy Markdown

Spike implementation of tree-sitter support for dart, further described in this issue: https://github.com/sourcegraph/sourcegraph/issues/57880

Test plan

@cla-bot

cla-bot Bot commented Oct 30, 2023

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

tree-sitter-rust = "0.20.3"
tree-sitter-typescript = "0.20.2"

tree-sitter-dart = { git = "https://github.com/matthewnitschke-wk/tree-sitter-dart", rev = "84440dc48288890dddb63345b6e1dd0168338c1a" }

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Had to create a fork of tree-sitter-dart due to a duplicate git submodule issue, and the reliance on tree-sitter 0.17

I think ideally this would be hosted by sourcegraph, similiar to other languages

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As luck would have it, @valerybugakov just fixed it :) UserNobody14/tree-sitter-dart#59

So this can be reverted back to upstream.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh! Fantastic! Thanks for looking

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, it looks like we're forking it as well: https://github.com/sourcegraph/tree-sitter-dart

I'll let Valery to speak to the reasons why, but I was only able to run tests on your PR using the forked grammar (the upstream failed to build the bindings)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry! Just seeing this now!

Awesome! I was running into that same problem, but couldn't spend the time figuring out what was going on. Was able to update the fork to sourcegraph's, and successfully generate the snapshots

@keynmol

keynmol commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

In terms of generating snapshots, I believe you can

  1. cargo test in sg-syntax
  2. cargo insta review

And then accept the snapshot, and check it in

@cla-bot

cla-bot Bot commented Nov 21, 2023

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

@cla-bot

cla-bot Bot commented Nov 21, 2023

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

@cla-bot

cla-bot Bot commented Nov 21, 2023

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement (CLA), and we don't have yours on file. In order for us to review and merge your code, please sign CLA to get yourself added.

Sourcegraph teammates should refer to Accepting contributions for guidance.

@matthewnitschke-wk

Copy link
Copy Markdown
Author

Closing in favor of https://github.com/sourcegraph/sourcegraph/pull/58480, which has signed cla

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants