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

locations: Add support for Spans with exact location of AST nodes #1480

Closed
wants to merge 3 commits into from

Conversation

lustefaniak
Copy link
Contributor

@lustefaniak lustefaniak commented Oct 22, 2024

@jakeswenson initially suggested this PR in #839. We are using it in SYNQ to drive our code-column lineage experience. I rebased it on the latest main.

After I rebased it to suggest this change, I noticed an ongoing effort in #1435.

If you consider the design suggested here for inclusion, I plan to add tests. We have an extensive internal test suite living in a downstream project using a fork of this library.

In our case, the accuracy of code locations is critical since we use it to drive the whole navigation experience. I like the WithSpan<T> approach better, as I know what element and where has accurate locations. In our original codebase not all Indents are wrapped in WithSpan but since this rebase effort was not a small thing I decided to replace all Ident with WithSpan<Ident>. Also to reduce future conflicts/PRs to add more locations.

TODO:

  • Add tests ensuring accurate location capture.
  • Remove empty_span() from parser. We didn't have some new nodes/quirks on our older fork.
  • Add additional WithSpan<T> where it makes sense, e.g. around Expr

@alamb
Copy link
Contributor

alamb commented Nov 30, 2024

Now that we have merged

I wonder if there is some way to reuse / get help testing spans in sqlarser-rs. Thoughts welcome on

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