Skip to content

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