locations: Add support for Spans with exact location of AST nodes #1480
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@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 allIndent
s are wrapped inWithSpan
but since this rebase effort was not a small thing I decided to replace allIdent
withWithSpan<Ident>
. Also to reduce future conflicts/PRs to add more locations.TODO:
empty_span()
from parser. We didn't have some new nodes/quirks on our older fork.WithSpan<T>
where it makes sense, e.g. aroundExpr