vscode-yaml icon indicating copy to clipboard operation
vscode-yaml copied to clipboard

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

Open leosh64 opened this issue 4 years ago • 12 comments

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)

leosh64 avatar Feb 19 '21 07:02 leosh64

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

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

tumido avatar May 22 '21 16:05 tumido

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

y120 avatar Dec 06 '21 00:12 y120

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 avatar Apr 20 '22 15:04 MarkusTeufelberger

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

ssbarnea avatar Apr 20 '22 16:04 ssbarnea

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

MarkusTeufelberger avatar Apr 21 '22 13:04 MarkusTeufelberger

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

ssbarnea avatar Apr 21 '22 14:04 ssbarnea

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.

linceaerian avatar Sep 02 '22 16:09 linceaerian

@linceaerian love it, attacking the conflict!

sdake avatar May 15 '23 03:05 sdake

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.

ssbarnea avatar May 15 '23 06:05 ssbarnea

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.

dmiller-tibco avatar Jan 18 '24 13:01 dmiller-tibco