google / yamlfmt

An extensible command line tool or library to format yaml files.
Apache License 2.0
1.2k stars 48 forks source link

Inconsistent formatting depending on platform: comment above list item moved below it on Windows #198

Open lovetheguitar opened 4 months ago

lovetheguitar commented 4 months ago

Hi,

thanks for the great library. :heart:

I found an inconsistency between a run on Ubuntu 22.04 and Windows 11. I run yamlfmt via pre-commit. In a second step I tried to pin the go-lang version by adding language_version: 1.22.5 to the hook definition.

`.pre-commit-config.yaml` **`.pre-commit-config.yaml`** ```yaml ️️️ - repo: https://github.com/google/yamlfmt rev: 5607b62b215d44fe90ca71a5329efff63a1a0d18 # frozen: v0.13.0 hooks: - id: yamlfmt language_version: 1.22.5 ``` Originally I was using those formatter arguments, but removed them for the output file below. ```yaml args: - -formatter - retain_line_breaks_single=true - -formatter - pad_line_comments=2 # two spaces before # comments, like in python - -formatter - scan_folded_as_literal=true # so > blocks will not get collapsed ``` ️️️

Diff

diff --git a/demo_yamlfmt_comment_swap.yaml b/demo_yamlfmt_comment_swap.yaml
index d042395..ba164a5 100644
--- a/demo_yamlfmt_comment_swap.yaml
+++ b/demo_yamlfmt_comment_swap.yaml
@@ -3,8 +3,8 @@ some_gitlab_ci_job:
     # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is defined and matches $CI_COMMIT_BRANCH -> add job
     - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH != null && $TAG_LATEST_COMMIT_SOURCE_BRANCH==$CI_COMMIT_BRANCH
       when: always
-    # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is not defined, and we run on $CI_DEFAULT_BRANCH -> add job
     - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH == null && $CI_DEFAULT_BRANCH == $CI_COMMIT_BRANCH
+      # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is not defined, and we run on $CI_DEFAULT_BRANCH -> add job
       when: always
-    # demo comment above when:never
     - when: never  # do not run otherwise
+      # demo comment above when:never
Before: demo_yamlfmt_comment_swap.yaml ️️️ **Input file** ```yaml some_gitlab_ci_job: rules: # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is defined and matches $CI_COMMIT_BRANCH -> add job - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH != null && $TAG_LATEST_COMMIT_SOURCE_BRANCH==$CI_COMMIT_BRANCH when: always # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is not defined, and we run on $CI_DEFAULT_BRANCH -> add job - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH == null && $CI_DEFAULT_BRANCH == $CI_COMMIT_BRANCH when: always # demo comment above when:never - when: never # do not run otherwise ``` ️️️
After: demo_yamlfmt_comment_swap.yaml ️️️ **Output file** ```yaml some_gitlab_ci_job: rules: # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is defined and matches $CI_COMMIT_BRANCH -> add job - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH != null && $TAG_LATEST_COMMIT_SOURCE_BRANCH==$CI_COMMIT_BRANCH when: always - if: $TAG_LATEST_COMMIT_SOURCE_BRANCH == null && $CI_DEFAULT_BRANCH == $CI_COMMIT_BRANCH # when $TAG_LATEST_COMMIT_SOURCE_BRANCH is not defined, and we run on $CI_DEFAULT_BRANCH -> add job when: always - when: never # do not run otherwise # demo comment above when:never ``` ️️️

The same file passes yamlfmt on linux in both versions.

Is there some workaround to have the same behavior cross platform?

braydonk commented 4 months ago

Hi @lovetheguitar thanks for opening an issue.

Interesting, I've seen the library be temperamental about scanning comment clusters in the past but never seen it do different things on different platforms.

I'm not sure if I'll be able to come up with a solution for this. I've tried to fix these kinds of weird comment placement bugs in the yaml library in the past and always came up short. This usually comes as a result of the way we have to hack retain_line_breaks to work, another thing I essentially can't fix in the library without rewriting it.

I'm sorry it's broken for this case, I would really like to fix it but I've never been able to.

Since you're already using pre-commit, I might recommend trying out this alternative: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt
I've not tried it myself as I'm not a pre-commit user, but it's possible that it's more accurate than this tool is.

lovetheguitar commented 4 months ago

Hey @braydonk,

thanks a lot for your fast response. Just had some time to play around with the alternative you recommended, but the maintainer sadly does not seem to care about windows support at all (https://github.com/jumanjihouse/pre-commit-hook-yamlfmt/issues?q=is%3Aissue+is%3Aopen+windows, https://github.com/jumanjihouse/pre-commit-hook-yamlfmt/pull/42).

So, for now we'll probably adopt your solution and live with this small inconsistency. Thanks again! (:

kenzht commented 1 month ago

v0.13.0 results in inconsistency, but v0.12.1 worked as expected on windows:)