-
Notifications
You must be signed in to change notification settings - Fork 510
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
Fix indentation of the closing quotes of a multiline string literal #1262
Fix indentation of the closing quotes of a multiline string literal #1262
Conversation
Closes pinterest#1127 Arranging the opening and closing quotes of multiline string literals should not be limited to strings followed by the ".trimIndent()" clause. In case the opening quotes have been placed at the start of the line, then the closing quotes should also be placed at the start of the line. In the reporter tests, the tab characters have replaced with an explicit placeholder "{TAB}" to make them visible.
ktlint-reporter-json/src/test/kotlin/com/pinterest/ktlint/reporter/json/JsonReporterTest.kt
Show resolved
Hide resolved
Hm, I'm a bit confused, the description doesn't look relevant to the PR itself, or does it? Also, what part of the diff actually fixes the closing quote? I only see changes relevant to the opening quote if it's placed on a newline so far... |
@@ -345,7 +345,7 @@ class IndentationRule : Rule("indent"), Rule.Modifier.RestrictToRootLast { | |||
val treeParent = n.treeParent | |||
if (treeParent.elementType == STRING_TEMPLATE) { | |||
val treeParentPsi = treeParent.psi as KtStringTemplateExpression | |||
if (treeParentPsi.isMultiLine() && treeParentPsi.isFollowedByTrimIndent() && n.treePrev.text.isNotBlank()) { | |||
if (treeParentPsi.isMultiLine() && n.treePrev.text.isNotBlank()) { |
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.
Why did we remove the check for trimIndent
?
Sorry about the confusion. I should have stated more clearly that I need input before being able to solve this entirely. I mentioned a debatable choice in the initial description. In one of the other discussion about indentation, you mentioned that you like to align the closing quotes with the opening quotes. This PR was aiming to solve that in the end. Currently the following is allowed:
How should the closing quotes be aligned in this case? Aligning would mean, the following.
I can not see this as an acceptable solution. Another solution, would be to wrap the opening quotes:
This is my preferred solution, but it can be considered as breaking with previous ktlint version. The removal of the trimIndent function is possible since we decided that indentation in multiline raw string literals is never changed. Therefore it is not relevant whether this literal is followed by trimIndent, trimMargin, anything else or nothing at all. It has no relation with the goal of the PR and should have been requested in a separate PR. If you answer my previous question about the placement of the closing quotes, then I will split this PR in separate PR's with a single change in each. |
# Conflicts: # ktlint-ruleset-standard/src/test/kotlin/com/pinterest/ktlint/ruleset/standard/IndentationRuleTest.kt
…otes # Conflicts: # ktlint-reporter-baseline/src/test/kotlin/com/pinterest/ktlint/reporter/baseline/BaselineReporterTest.kt
Fix indentation below:
to
However, above might be a debatable choice. Therefore I left following TODO in a test:
I would like your opinion about it, so that I can finalize this PR.
This changes also includes the fix for #1127 as it has a comparable problem with the closing quotes.