-
-
Notifications
You must be signed in to change notification settings - Fork 655
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
Push indents to after comments #4239
Conversation
Pull Request Test Coverage Report for Build 3859330151
💛 - Coveralls |
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## main #4239 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 200 200
Lines 15143 15148 +5
=========================================
+ Hits 15143 15148 +5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
test_fail_coverage_indent_trough: | ||
# This test primarily tests the handling of closing trough indents | ||
fail_str: | | ||
WITH bar as (SELECT 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the past, L003 only handled indentation. Any concerns that here it is moving code between lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so - this is functionality that is already out in the wild in the treatment of existing hanging indents in 2.0.0a1
. All this PR does is fix a bug with it.
This resolves #4223 . This fix is a little unorthodox - but it works.
When assessing the position of indent effects, we push the effects of any
Indent
orDedent
segments to after any comment segments. Within a line it shouldn't make a difference, but for end of line comments, it makes sure that any tokens are available in the sameIndentPoint
that forms the start of the next line. That's the main purpose of what we're doing.I considered actually moving the
Indent
andDedent
tokens in the lexing or parsing stage, but that felt like too much "hacking". Keeping this logic in the reflow interpretation code feels acceptable.