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 formatting not compliant with yamllint "spaces before comments" requirement #433

Open
1 of 4 tasks
leosh64 opened this issue Feb 19, 2021 · 12 comments
Open
1 of 4 tasks

Comments

@leosh64
Copy link

leosh64 commented Feb 19, 2021

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
  • Linux
  • other (please specify)
@tumido
Copy link
Member

tumido commented May 22, 2021

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

  1. YAML for VSCode uses Prettier so we first need support for this in Prettier. Please 👍 on this PR if you can: [YAML] Allow to set amount of spaces between comments and content prettier/prettier#10926
  2. We can update Prettier and enable the flag in here (feat(prettier): Support doNotIndent and commentSpacesFromContent #519, feat(prettier): Support doNotIndent and commentSpacesFromContent yaml-language-server#471)

@y120
Copy link

y120 commented Dec 6, 2021

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

@MarkusTeufelberger
Copy link

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).

MarkusTeufelberger pushed a commit to mgit-at/ansible-collection-roles that referenced this issue Apr 20, 2022
MarkusTeufelberger pushed a commit to mgit-at/ansible-collection-roles that referenced this issue Apr 20, 2022
@ssbarnea
Copy link
Member

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
Copy link

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
Copy link
Member

@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
Copy link

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
Copy link

sdake commented May 15, 2023

@linceaerian love it, attacking the conflict!

@ssbarnea
Copy link
Member

ssbarnea commented May 15, 2023

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
Copy link

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
Copy link

bdsoha commented May 15, 2024

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

@triwats
Copy link

triwats commented Aug 8, 2024

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

  1. Create .yamllint.yml in the parent directory
  2. 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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests