YAML formatting not compliant with yamllint "spaces before comments" requirement

leosh64 commented 3 years ago

Describe the bug

yamllint, a linter used by many projects and companies, has a different requirement on the number of spaces before comments than what the YAML formatter in this plugin provides:

What yamllint requires is two spaces before the comment #:

    timeout: 1800  # value in seconds ~ 30m

Formatting of this plugin results in a single space:

    timeout: 1800 # value in seconds ~ 30m

Thus, YAML files saved (with Format on Save activated) with VS Code and this plugin enabled results in YAML files that are rejected by systems using yamllint to check for compliance (e.g. CI systems).

Expected Behavior

Files are formatted according to the yamllint requirements, e.g. double space before comments.

OR

Option to specify formatting requirements is given.

Current Behavior

Files are not formatted according to yamllint requirements, e.g. single space before comments.

Steps to Reproduce

Create file test.yaml with contents:
job: name: some-job timeout: 1800 # value in seconds ~ 30m
Run yamllint test.yaml --> pass
Apply formatting in VS Code which will reduce double-space to single-space
Run yamllint test.yaml --> fail

Environment

[ ] Windows
[ ] Mac
[x] Linux
[ ] other (please specify)

tumido commented 3 years ago

Hey! So I've digged this rabbit hole...

YAML for VSCode uses Prettier so we first need support for this in Prettier. Please :+1: on this PR if you can: https://github.com/prettier/prettier/pull/10926
We can update Prettier and enable the flag in here (https://github.com/redhat-developer/vscode-yaml/pull/519, https://github.com/redhat-developer/yaml-language-server/pull/471)

y120 commented 2 years ago

prettier has signalled intent to reject the linked PR. Is using a different formatter an option? This makes vscode-yaml unusable for me.

MarkusTeufelberger commented 2 years ago

Same here, I installed the Ansible VS Code extension and now my CI breaks if I use the (default-on!) YAML formatter. Completely disabling it is also not very nice, but apparently necessary. As this is apparently known for over a year, I really wonder why it still happens that a YAML extension creates YAML files that don't pass the most basic linter out there (yamllint).

Personally I also prefer the "2 spaces before inline comments" style by the way, so please fork prettier or use a different formatter (optionally, if necessary).

ssbarnea commented 2 years ago

I am pretty sure we will not fork prettier to add this (maintenance efforts are huge, this being only one of the issues). In the end I had to personally change ansible-lint defaults in v6 to match prettier and diverge from older yamllint defaults.

I know very well that black has a two-chars before comment, and that was likely the reason why yamllint opted for the same default.

While talking with others and trying to explain why two spaces are better instead of one, I discovered that on YAML there is not really a need for this, or at least is considerably lower than on python. On python it was problematic especially on long lines or lines that had expressions, but on yaml you usually don't have this. The coloring also helps distinguish the lines. Most of yaml comments are not inline and are before the affected line.

The practical workaround for this problem is to add one line to your yamllint config, like https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/yaml_utils.py#L54

MarkusTeufelberger commented 2 years ago

Well, that workaround would then break on every single file that wasn't made prettier...

I also disagree about the single space, I prefer the slight offset. To be pedantic, even the YAML spec only mentions "white space characters" (note the plural!) for comments: https://yaml.org/spec/1.2.2/#66-comments Python code can also be colored and non-inline comments are out of scope anyways.

Well, I guess either there needs to be a better autoformatter than prettier for this extension or the shortcomings of prettier need to be better documented (such as requiring a custom yamllint config everywhere it is used). I also found it weird that prettier seems to have no opinion regarding boolean values by the way compared to the schemas for ansible (but not ansible-lint).

ssbarnea commented 2 years ago

@MarkusTeufelberger you forgot that prettier is not a linter, it is just a formatter. If you do not like its behavior I would invite you to raise a bug with it.

Based on popularity of prettier (42k stars) versus yamllint (1.9k) and the fact that this project has zero python code in it, I would say that what yamllint does by default, does not matter so much.

On the other hand we do have an example where yamllint (sole) maintainer refused to use indented sequences in its default settings, even if they are more popular around. So you do already have to change yamllint config if you want to also use it.

Keep in mind that this extension has nothing to do with Ansible or Python. Shortly if you don't like its formatting, disable it. I doubt we see a change in this area on the foreseeable future, especially as I am aware of far more pressing bugs that need to be addressed.

linceaerian commented 2 years ago

Hello, not a definitive solution but a workaround to use "prettier" format with https://yamllint.readthedocs.io/en/stable/rules.html#module-yamllint.rules.comments

Create a .yamllint
Add a custom rule:
```
comments:
min-spaces-from-content: 1
```

It will do the job of "not breaking the yamlint" but will still use a spacing of 1.

sdake commented 1 year ago

@linceaerian love it, attacking the conflict!

ssbarnea commented 1 year ago

yamllint has two presets, a default one and a relaxed one if i remember well. I proposed addition of another "prettier" format few months back but we might need some community backing to make it happen. Yamllint project is single-person project and in cases like this is extra hard to make such a case as its opinionation part is directly dependent on that.

Projects with multiple maintainers are more likely to bend towards where the users are asking for.

So, look for open tickets and comment on them. The ball is on yamllint and prettier court IMHO, not here.

dmiller-tibco commented 10 months ago

I am removing prettier from vscode due to this bug. yamllint for helm charts without a custom config is a chart publishing standard and prettier formatting needs to be compliant, or have an option to be for it to be useable.

bdsoha commented 6 months ago

Why has no headway been made with this issue? So many people are having the same issue!

triwats commented 3 months ago

Just to follow on from what @linceaerian came up with, here is how I've gotten around this as a full solution as unfortunately this falls between the philosophy of our modern editor (vscode) and the standards associated with prettier and yamllint

Create .yamllint.yml in the parent directory
Add the following block:

---
rules:
  comments:
    min-spaces-from-content: 1 # Changed this to stop a mess between linters from Prettier (vscode) to yamllint - https://github.com/prettier/prettier/pull/10926

Hope this helps someone!

redhat-developer / vscode-yaml