Skip to content
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

YAML: Fix printing doubles a blank line before a comment #9143

Merged
merged 12 commits into from Sep 7, 2020
27 changes: 27 additions & 0 deletions changelog_unreleased/yaml/pr-9143.md
@@ -0,0 +1,27 @@
#### Fix printing doubles a blank line before a comment (#9143 by @sosukesuzuki)

<!-- prettier-ignore -->
```yaml
# Input
- foo: 0
bar: 1

# baz: 2
- quux: 3

# Prettier stable
- foo: 0
bar: 1


# baz: 2
- quux: 3

# Prettier master
- foo: 0
bar: 1

# baz: 2
- quux: 3

```
14 changes: 11 additions & 3 deletions src/language-yaml/printer-yaml.js
Expand Up @@ -127,8 +127,7 @@ function genericPrint(path, options, print) {
])
)
: "",
nextEmptyLine,
hasEndComments(node) && !isNode(node, ["documentHead", "documentBody"])
shouldPrintEndComments(node)
? align(
node.type === "sequenceItem" ? 2 : 0,
concat([
Expand All @@ -153,6 +152,7 @@ function genericPrint(path, options, print) {
])
)
: "",
nextEmptyLine,
]);
}

Expand Down Expand Up @@ -703,6 +703,12 @@ function needsSpaceInFrontOfMappingValue(node) {
return node.key.content && node.key.content.type === "alias";
}

function shouldPrintEndComments(node) {
return (
hasEndComments(node) && !isNode(node, ["documentHead", "documentBody"])
);
}

function printNextEmptyLine(path, originalText) {
const node = path.getValue();
const root = path.stack[0];
Expand All @@ -713,7 +719,9 @@ function printNextEmptyLine(path, originalText) {
if (!root.isNextEmptyLinePrintedChecklist[node.position.end.line]) {
if (isNextLineEmpty(node, originalText)) {
root.isNextEmptyLinePrintedChecklist[node.position.end.line] = true;
return softline;
if (!shouldPrintEndComments(path.getParentNode())) {
return softline;
}
}
}

Expand Down
202 changes: 202 additions & 0 deletions tests/yaml/comment/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -137,6 +137,44 @@ printWidth: 80
================================================================================
`;

exports[`issue-9130.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
printWidth: 80
| printWidth
=====================================input======================================
- foo: 0
bar: 1

# baz: 2
- quux: 3

- foo: 0
bar: 1

# baz: 2

# baz: 3
- quux: 3

=====================================output=====================================
- foo: 0
bar: 1

# baz: 2
- quux: 3

- foo: 0
bar: 1

# baz: 2

# baz: 3
- quux: 3

================================================================================
`;

exports[`map.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
Expand Down Expand Up @@ -282,6 +320,69 @@ foo2:
================================================================================
`;

exports[`map-4.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
printWidth: 80
| printWidth
=====================================input======================================
before:

# before.comment
after:
# after.comment

before-after:

# before-after.comment

none:
# none.comment
before(2 line):


# before.comment
after(2 line):
# after.comment


before-after(2 line):


# before-after.comment


none(2):
# none.comment
=====================================output=====================================
before:

# before.comment
after:
# after.comment

before-after:

# before-after.comment

none:
# none.comment
before(2 line):

# before.comment
after(2 line):
# after.comment

before-after(2 line):

# before-after.comment

none(2):
# none.comment

================================================================================
`;

exports[`object.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
Expand Down Expand Up @@ -336,6 +437,107 @@ printWidth: 80
================================================================================
`;

exports[`sequence.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
printWidth: 80
| printWidth
=====================================input======================================
- - a

# - b

# - c

- e

- - a

# - b

# - c

=====================================output=====================================
- - a

# - b

# - c

- e

- - a

# - b

# - c

================================================================================
`;

exports[`sequence-2.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
printWidth: 80
| printWidth
=====================================input======================================
- before

# before.comment
- after
# after.comment

- before-after

# before-after.comment

- none:
# none.comment
- before(2 line)


# before.comment
- after(2 line)
# after.comment


- before-after(2 line)


# before-after.comment


- none(2)
# none.comment
=====================================output=====================================
- before

# before.comment
Comment on lines +513 to +515
Copy link
Sponsor Member

@fisker fisker Sep 7, 2020

Choose a reason for hiding this comment

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

This is strange!!!

Prettier pr-9143
Playground link

--parser yaml

Input:

foo:
  # comment
bar:
  # comment

Output:

foo:
  # comment
bar:
  # comment

Prettier pr-9143
Playground link

--parser yaml

Input:

foo:
  # comment
barr:
  # comment

Output:

foo:
# comment   <---  this comment didn't indent, next key is `barr`
barr:
  # comment

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Ah, seems attaching logic issue. I'll open an issue in the parser.

Copy link
Sponsor Member

@fisker fisker Sep 7, 2020

Choose a reason for hiding this comment

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

Issue in yaml-unist-parser prettier/yaml-unist-parser#275

- after
# after.comment

- before-after

# before-after.comment

- none:
# none.comment
- before(2 line)

# before.comment
- after(2 line)
# after.comment

- before-after(2 line)

# before-after.comment

- none(2)
# none.comment

================================================================================
`;

exports[`set.yml format 1`] = `
====================================options=====================================
parsers: ["yaml"]
Expand Down
13 changes: 13 additions & 0 deletions tests/yaml/comment/issue-9130.yml
@@ -0,0 +1,13 @@
- foo: 0
bar: 1

# baz: 2
- quux: 3

- foo: 0
bar: 1

# baz: 2

# baz: 3
- quux: 3
28 changes: 28 additions & 0 deletions tests/yaml/comment/map-4.yml
@@ -0,0 +1,28 @@
before:

# before.comment
after:
# after.comment

before-after:

# before-after.comment

none:
# none.comment
before(2 line):


# before.comment
after(2 line):
# after.comment


before-after(2 line):


# before-after.comment


none(2):
# none.comment
28 changes: 28 additions & 0 deletions tests/yaml/comment/sequence-2.yml
@@ -0,0 +1,28 @@
- before

# before.comment
- after
# after.comment

- before-after

# before-after.comment

- none:
# none.comment
- before(2 line)


# before.comment
- after(2 line)
# after.comment


- before-after(2 line)


# before-after.comment


- none(2)
# none.comment
13 changes: 13 additions & 0 deletions tests/yaml/comment/sequence.yml
@@ -0,0 +1,13 @@
- - a

# - b

# - c

- e

- - a

# - b

# - c