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

Add spans to the Selector AST #1783

Closed
nex3 opened this issue Aug 18, 2022 · 2 comments · Fixed by #1903
Closed

Add spans to the Selector AST #1783

nex3 opened this issue Aug 18, 2022 · 2 comments · Fixed by #1903
Labels
AST API Issues about the sass_api Dart package enhancement

Comments

@nex3
Copy link
Contributor

nex3 commented Aug 18, 2022

Currently the selector AST used by Sass and exposed by sass_api doesn't expose source span information. This information is tricky to determine, because selectors are interpolated in the source file. However, it should be possible to map the source spans from the parsed string back to the original file by tracking where the source offsets of the interpolated sections begin and end, and thereby get accurate spans for the selectors.

This would allow us to improve a number of error/deprecation messages, and make the Kythe indexer implementation cleaner.

@nex3 nex3 added enhancement AST API Issues about the sass_api Dart package labels Aug 18, 2022
@stof
Copy link
Contributor

stof commented Sep 3, 2022

Even without taking interpolation into account, complex and compound selectors won't have a single span as source (due to the nesting features, and mixins might even make them combine simple selectors from different files). Note that the &-suffix feature also means that even simple selectors could have multiple source spans.

@nex3
Copy link
Contributor Author

nex3 commented Sep 6, 2022

For nested selectors, we'd generally fall back to using the innermost span when necessary. You're right that that does increase the complexity of the translation, though.

nex3 added a commit that referenced this issue Mar 3, 2023
nex3 added a commit that referenced this issue Mar 6, 2023
nex3 added a commit that referenced this issue Mar 7, 2023
nex3 added a commit to sass/sass-spec that referenced this issue Mar 7, 2023
nex3 added a commit that referenced this issue Mar 7, 2023
nex3 added a commit that referenced this issue Mar 7, 2023
nex3 added a commit to sass/sass-spec that referenced this issue Mar 8, 2023
nex3 added a commit to sass/sass-spec that referenced this issue Mar 8, 2023
@nex3 nex3 closed this as completed in #1903 Mar 8, 2023
nex3 added a commit that referenced this issue Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AST API Issues about the sass_api Dart package enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants