From ea778a7e7a96d853791e3fd48d392f5477fc65ae Mon Sep 17 00:00:00 2001 From: Daniel Chao Date: Fri, 31 Oct 2025 10:47:53 -0700 Subject: [PATCH] Don't force multiline interpolation into a single line (#1280) Also, fixes an issue where forced single-line formatting would break for if/else and let expressions --------- Co-authored-by: Islon Scherer --- docs/modules/release-notes/pages/0.30.adoc | 2 +- .../main/kotlin/org/pkl/formatter/Builder.kt | 26 +++++++++--------- .../input/string-interpolation.pkl | 27 ++++++++++++++++++- .../output/multi-line-strings.pkl | 4 ++- .../output/string-interpolation.pkl | 26 ++++++++++++++++++ 5 files changed, 69 insertions(+), 16 deletions(-) diff --git a/docs/modules/release-notes/pages/0.30.adoc b/docs/modules/release-notes/pages/0.30.adoc index b03e2154..1a5cabb4 100644 --- a/docs/modules/release-notes/pages/0.30.adoc +++ b/docs/modules/release-notes/pages/0.30.adoc @@ -25,7 +25,7 @@ To get started, follow xref:pkl-cli:index.adoc#installation[Installation].# [[formatter]] === Formatter -Pkl now has a formatter (https://github.com/apple/pkl/pull/1107[#1107], https://github.com/apple/pkl/pull/1208[#1208], https://github.com/apple/pkl/pull/1211[#1211], https://github.com/apple/pkl/pull/1215[#1215], https://github.com/apple/pkl/pull/1217[#1217], https://github.com/apple/pkl/pull/1235[#1235], https://github.com/apple/pkl/pull/1246[#1246], https://github.com/apple/pkl/pull/1247[#1247], https://github.com/apple/pkl/pull/1252[#1252], https://github.com/apple/pkl/pull/1256[#1256], https://github.com/apple/pkl/pull/1259[#1259], https://github.com/apple/pkl/pull/1260[#1260], https://github.com/apple/pkl/pull/1263[#1263], https://github.com/apple/pkl/pull/1265[#1265], https://github.com/apple/pkl/pull/1266[#1266], https://github.com/apple/pkl/pull/1267[#1267], https://github.com/apple/pkl/pull/1270[#1270], https://github.com/apple/pkl/pull/1271[#1271], https://github.com/apple/pkl/pull/1272[#1272], https://github.com/apple/pkl/pull/1273[#1273], https://github.com/apple/pkl/pull/1274[#1274])! +Pkl now has a formatter (https://github.com/apple/pkl/pull/1107[#1107], https://github.com/apple/pkl/pull/1208[#1208], https://github.com/apple/pkl/pull/1211[#1211], https://github.com/apple/pkl/pull/1215[#1215], https://github.com/apple/pkl/pull/1217[#1217], https://github.com/apple/pkl/pull/1235[#1235], https://github.com/apple/pkl/pull/1246[#1246], https://github.com/apple/pkl/pull/1247[#1247], https://github.com/apple/pkl/pull/1252[#1252], https://github.com/apple/pkl/pull/1256[#1256], https://github.com/apple/pkl/pull/1259[#1259], https://github.com/apple/pkl/pull/1260[#1260], https://github.com/apple/pkl/pull/1263[#1263], https://github.com/apple/pkl/pull/1265[#1265], https://github.com/apple/pkl/pull/1266[#1266], https://github.com/apple/pkl/pull/1267[#1267], https://github.com/apple/pkl/pull/1270[#1270], https://github.com/apple/pkl/pull/1271[#1271], https://github.com/apple/pkl/pull/1272[#1272], https://github.com/apple/pkl/pull/1273[#1273], https://github.com/apple/pkl/pull/1274[#1274], https://github.com/apple/pkl/pull/1280[#1280])! Pkl's formatter is _canonical_, which means that it has a single set of formatting rules, with (almost) no configuration options. The goal is to eliminate the possibility of formatting debates, which can lead to churn and bike-shedding. diff --git a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt index 8d039988..68c5ab91 100644 --- a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt +++ b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt @@ -39,8 +39,6 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe private var prevNode: Node? = null private var noNewlines = false - private class CannotAvoidNewline : RuntimeException() - fun format(node: Node): FormatNode { prevNode = node return when (node.type) { @@ -836,16 +834,10 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe while (cursor.hasNext()) { if (isInStringInterpolation) { val prevNoNewlines = noNewlines - noNewlines = true val elems = cursor.takeUntilBefore { it.isTerminal(")") } + noNewlines = !elems.isMultiline() getBaseSeparator(prev!!, elems.first())?.let { add(it) } - val formatted = - try { - formatGeneric(elems, null) - } catch (_: CannotAvoidNewline) { - noNewlines = false - formatGeneric(elems, null) - } + val formatted = formatGeneric(elems, null) addAll(formatted) getBaseSeparator(elems.last(), cursor.peek())?.let { add(it) } noNewlines = prevNoNewlines @@ -872,7 +864,7 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe } private fun formatIf(node: Node): FormatNode { - val separator = if (node.isMultiline()) forceLine() else spaceOrLine() + val separator = if (node.isMultiline()) forceSpaceyLine() else spaceOrLine() val nodes = formatGeneric(node.children) { _, next -> // produce `else if` in the case of nested if. @@ -971,7 +963,7 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe } private fun formatLetExpr(node: Node): FormatNode { - val separator = if (node.isMultiline()) forceLine() else spaceOrLine() + val separator = if (node.isMultiline()) forceSpaceyLine() else spaceOrLine() val endsWithLet = node.children.last().type == NodeType.LET_EXPR val nodes = formatGenericWithGen( @@ -1315,7 +1307,12 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe } private fun mustForceLine(): FormatNode { - return if (noNewlines) throw CannotAvoidNewline() else ForceLine + if (noNewlines) { + // should never happen; we do not set `noNewlines` for interpolation blocks that span multiple + // lines + throw RuntimeException("Tried to render Pkl code as single line") + } + return ForceLine } private fun forceLine(): FormatNode { @@ -1460,6 +1457,9 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe private fun Node.isMultiline(): Boolean = span.lineBegin < span.lineEnd + private fun List.isMultiline(): Boolean = + if (isEmpty()) false else first().span.lineBegin < last().span.lineEnd + private inline fun List.splitOn(pred: (T) -> Boolean): Pair, List> { val index = indexOfFirst { pred(it) } return if (index == -1) { diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/input/string-interpolation.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/input/string-interpolation.pkl index ad2a36c9..f543d0ea 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/input/string-interpolation.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/input/string-interpolation.pkl @@ -13,7 +13,7 @@ prop3 = \(new { // some comment foo = 1 - + // some comment bar = 2 }) @@ -49,3 +49,28 @@ prop7 = "\( )" prop8 = "\(new { foo = 1 bar = 2 baz = 3 })" + +prop9 = "\(if (true) 1 else 2)" + +prop10 = "\( + if (true) 1 + else 2 +)" + +prop11 = "\( + new { + 1; + 2; + 3; + } +)" + +// single line expressions are not broken up +prop12 = "Some \(if (true) 1 else 2) reeeaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaallly long string with interpolation" + +// multi-line expressions are preserved +prop13 = "Some \( + if (true) + 1 + else 2 + ) reeeaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaallly long string with interpolation" diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/multi-line-strings.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/multi-line-strings.pkl index 35246e0c..908dcfa8 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/output/multi-line-strings.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/multi-line-strings.pkl @@ -1,6 +1,8 @@ foo = """ - asd \(new { bar = 1 }) asd + asd \(new { + bar = 1 + }) asd """ bar = diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/string-interpolation.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/string-interpolation.pkl index b78d7eab..33498b22 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/output/string-interpolation.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/string-interpolation.pkl @@ -50,3 +50,29 @@ prop7 = )" prop8 = "\(new { foo = 1; bar = 2; baz = 3 })" + +prop9 = "\(if (true) 1 else 2)" + +prop10 = + "\(if (true) + 1 + else + 2)" + +prop11 = + "\(new { + 1 + 2 + 3 + })" + +// single line expressions are not broken up +prop12 = + "Some \(if (true) 1 else 2) reeeaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaallly long string with interpolation" + +// multi-line expressions are preserved +prop13 = + "Some \(if (true) + 1 + else + 2) reeeaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaallly long string with interpolation"