Skip to content

Track Parens<T>'s span#2291

Open
xitep wants to merge 7 commits intoapache:mainfrom
xitep:parens
Open

Track Parens<T>'s span#2291
xitep wants to merge 7 commits intoapache:mainfrom
xitep:parens

Conversation

@xitep
Copy link
Copy Markdown
Contributor

@xitep xitep commented Mar 31, 2026

  • this suggests Parens<T>. hopefully advancing [EPIC] Complete Span (source location) information / feature #1548 a bit and being the foundation for Inconsistent spans for function calls #2050
  • it is (a bit) wasteful to store the parens' location as a Span (since a paren is just a single character, the start location would fully suffice); i simply decided for consistency with the rest of the AST.
  • with this PR (so far), it's used only by Value so that i can properly determine the end of an insert statement and distinguish between "... VALUES ('foo' /\*xyz\*/) and ... VALUES('foo') /\*xyz\*/
  • of course, many more places in the AST could be covered, e.g. function params/args, subqueries, etc. if this PR is OK as is, i can put in some more time and expand on the coverage. (just let me know.)

@xitep xitep changed the title Parens Track Parens<T>'s span Mar 31, 2026
@xitep xitep force-pushed the parens branch 3 times, most recently from 6620b80 to 88cd0d7 Compare April 9, 2026 13:22
Copy link
Copy Markdown
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @xitep!

@iffyio iffyio added this pull request to the merge queue Apr 17, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 17, 2026
@xitep
Copy link
Copy Markdown
Contributor Author

xitep commented Apr 17, 2026

i guess, we'll wait for #2308.

@iffyio
Copy link
Copy Markdown
Contributor

iffyio commented Apr 17, 2026

@xitep the clippy issues have been fixed on main now, could you rebase when you get the chance?

@xitep
Copy link
Copy Markdown
Contributor Author

xitep commented Apr 17, 2026

alright @iffyio, looks like it's good now :) (btw: thank you for the fix-clippy-pr!)

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.

2 participants