diff --git a/changelog_unreleased/yaml/10874.md b/changelog_unreleased/yaml/10874.md new file mode 100644 index 000000000..c73e119ec --- /dev/null +++ b/changelog_unreleased/yaml/10874.md @@ -0,0 +1,66 @@ +#### Don't switch to explicit mappings needlessly (#10874 by @pdavies) + +Previously, Prettier printed long YAML keys using explicit mapping syntax. The intent was to use that syntax to accommodate multiline keys, but it had the unintended effect of using it even for long keys that couldn’t be made multiline – for example, because `--prose-wrap` was set to `preserve` or the key didn’t contain whitespace. This is bad because: + +- Doing so has the effect of further increasing the line length. +- Explicit mappings are obscure and most people have never seen them. It causes confusion to introduce them needlessly. + + +```yaml +# Input +- stage: Process + jobs: + - template: Process.yml + parameters: + ${{ if in(parameters.BuildType, 'a') }}: + BuildArtifacts: + - input: Build.Release.x86 + output: Processed.Release.x86 + ${{ if in(parameters.BuildType, 'b ') }}: + BuildArtifacts: + - input: Build.Release + output: Processed.Release + +# Prettier stable +- stage: Process + jobs: + - template: Process.yml + parameters: + ${{ if in(parameters.BuildType, 'a') }}: + BuildArtifacts: + - input: Build.Release.x86 + output: Processed.Release.x86 + ? ${{ if in(parameters.BuildType, 'b ') }} + : BuildArtifacts: + - input: Build.Release + output: Processed.Release + +# Prettier main +- stage: Process + jobs: + - template: Process.yml + parameters: + ${{ if in(parameters.BuildType, 'a') }}: + BuildArtifacts: + - input: Build.Release.x86 + output: Processed.Release.x86 + ${{ if in(parameters.BuildType, 'b ') }}: + BuildArtifacts: + - input: Build.Release + output: Processed.Release + +# Prettier main (with --prose-wrap always) +- stage: Process + jobs: + - template: Process.yml + parameters: + ${{ if in(parameters.BuildType, 'a') }}: + BuildArtifacts: + - input: Build.Release.x86 + output: Processed.Release.x86 + ? ${{ if in(parameters.BuildType, 'b ') + }} + : BuildArtifacts: + - input: Build.Release + output: Processed.Release +``` diff --git a/src/language-yaml/print/mapping-item.js b/src/language-yaml/print/mapping-item.js index 7a23e14fc..d669fc350 100644 --- a/src/language-yaml/print/mapping-item.js +++ b/src/language-yaml/print/mapping-item.js @@ -88,9 +88,17 @@ function printMappingItem(node, parentNode, path, print, options) { ifBreak("? "), group(alignWithSpaces(2, printedKey), { id: groupId }), ]); - const breakValue = [hardline, ": ", alignWithSpaces(2, printedValue)]; + + // Construct both explicit and implicit mapping values. + const explicitMappingValue = [ + hardline, + ": ", + alignWithSpaces(2, printedValue), + ]; /** @type {Doc[]} */ - const flatValueParts = [spaceBeforeColon, ":"]; + // In the implicit case, it's convenient to treat everything from the key's colon + // as part of the mapping value + const implicitMappingValueParts = [spaceBeforeColon, ":"]; if ( hasLeadingComments(value.content) || (hasEndComments(value) && @@ -103,15 +111,34 @@ function printMappingItem(node, parentNode, path, print, options) { value.content.tag === null && value.content.anchor === null) ) { - flatValueParts.push(hardline); + implicitMappingValueParts.push(hardline); } else if (value.content) { - flatValueParts.push(line); + implicitMappingValueParts.push(line); } - flatValueParts.push(printedValue); - const flatValue = alignWithSpaces(options.tabWidth, flatValueParts); + implicitMappingValueParts.push(printedValue); + const implicitMappingValue = alignWithSpaces( + options.tabWidth, + implicitMappingValueParts + ); + // If a key is definitely single-line, forcibly use implicit style to avoid edge cases (very long + // keys) that would otherwise trigger explicit style as if it was multiline. + // In those cases, explicit style makes the line even longer and causes confusion. + if ( + isAbsolutelyPrintedAsSingleLineNode(key.content, options) && + !hasLeadingComments(key.content) && + !hasMiddleComments(key.content) && + !hasEndComments(key) + ) { + return conditionalGroup([[printedKey, implicitMappingValue]]); + } + + // Use explicit mapping syntax if the key breaks, implicit otherwise return conditionalGroup([ - [groupedKey, ifBreak(breakValue, flatValue, { groupId })], + [ + groupedKey, + ifBreak(explicitMappingValue, implicitMappingValue, { groupId }), + ], ]); } @@ -127,6 +154,7 @@ function isAbsolutelyPrintedAsSingleLineNode(node, options) { break; case "alias": return true; + default: return false; } diff --git a/tests/format/yaml/comment/__snapshots__/jsfmt.spec.js.snap b/tests/format/yaml/comment/__snapshots__/jsfmt.spec.js.snap index 40c5720ed..db44b1163 100644 --- a/tests/format/yaml/comment/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/yaml/comment/__snapshots__/jsfmt.spec.js.snap @@ -6,9 +6,9 @@ parsers: ["yaml"] printWidth: 80 | printWidth =====================================input====================================== -a: +a: # a.trailingComment 123 - # impicitMappginValue + # implicitMappingValue ? b # explicitMappingKey @@ -37,9 +37,9 @@ empty_content: # hello world =====================================output===================================== -a: +a: # a.trailingComment 123 - # impicitMappginValue + # implicitMappingValue ? b # explicitMappingKey diff --git a/tests/format/yaml/comment/collection.yml b/tests/format/yaml/comment/collection.yml index c93a08322..f3df4914f 100644 --- a/tests/format/yaml/comment/collection.yml +++ b/tests/format/yaml/comment/collection.yml @@ -1,6 +1,6 @@ -a: +a: # a.trailingComment 123 - # impicitMappginValue + # implicitMappingValue ? b # explicitMappingKey diff --git a/tests/format/yaml/mapping/__snapshots__/jsfmt.spec.js.snap b/tests/format/yaml/mapping/__snapshots__/jsfmt.spec.js.snap index 37b1da0b2..ffb14a8ab 100644 --- a/tests/format/yaml/mapping/__snapshots__/jsfmt.spec.js.snap +++ b/tests/format/yaml/mapping/__snapshots__/jsfmt.spec.js.snap @@ -366,11 +366,25 @@ tabWidth: 4 : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong : value +? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong +: # Comment + foo: bar +? multiline + scalar + key +: value =====================================output===================================== key1: value key2: longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: value +solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: + # Comment + foo: bar +? multiline + scalar + key +: value ================================================================================ `; @@ -387,11 +401,25 @@ printWidth: 80 : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong : value +? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong +: # Comment + foo: bar +? multiline + scalar + key +: value =====================================output===================================== key1: value key2: longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: value +solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong: + # Comment + foo: bar +? multiline + scalar + key +: value ================================================================================ `; diff --git a/tests/format/yaml/mapping/explicit-key.yml b/tests/format/yaml/mapping/explicit-key.yml index 3223ef8eb..1f81c1d42 100644 --- a/tests/format/yaml/mapping/explicit-key.yml +++ b/tests/format/yaml/mapping/explicit-key.yml @@ -4,3 +4,10 @@ : longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong ? longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong : value +? solongitshouldbreakbutitcannot_longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong +: # Comment + foo: bar +? multiline + scalar + key +: value