yamlfmt: Doublestar excludes doesn't seem to work:

I have this:

doublestar: true
exclude:
  - **/templates/**
formatter:
  type: basic
  indent: 2
  include_document_start: true

And I am doing this to prevent the yamlfmt from formatting my templates/ folder that are in my helm charts, since it messes everything up.

However, it keeps complaining that: 2023/03/24 14:38:02 yaml: line 3: did not find expected alphabetic or numeric character.

So I put line 3 in quotes "- **/templates/**" and now the error goes away. However, it doesn’t respect my excludes and continues to format things in the templates/ folder.

About this issue

Original URL
State: closed
Created a year ago
Comments: 25 (5 by maintainers)

Commits related to this issue

Add debug logs for https://github.com/google/yamlfmt/issues/97 — committed to badouralix/google-yamlfmt by badouralix a year ago
yamlfmt_test: re-enable and fix doublestar tests Doublestar tests have been disabled for quite a long time and I honestly forget why they were. Regardless, they were missing some features to be prope... — committed to braydonk/yamlfmt by braydonk a year ago
yamlfmt_test: re-enable and fix doublestar tests Doublestar tests have been disabled for quite a long time and I honestly forget why they were. Regardless, they were missing some features to be prope... — committed to braydonk/yamlfmt by braydonk a year ago
yamlfmt_test: re-enable and fix doublestar tests (#111) Doublestar tests have been disabled for quite a long time and I honestly forget why they were. Regardless, they were missing some features to ... — committed to google/yamlfmt by braydonk a year ago

Most upvoted comments

Thanks for the help working through this @badouralix I have been sick and not looking at GitHub this weekend.

The change to absolute path was because previously absolute paths wouldn’t match on every circumstance. If the collected path was like this:

data/k8s/charts/chart.yaml

And the exclude path was like this:

/Users/someone/data/charts/k8s/**/*.yaml

Then it would think the collected path is not a match for the exclusion pattern. The only way that made sense was to get the absolute path of the collected path. What I didn’t realize is that I just inverted the problem.

This prompted me to take a look, and I realized that I have been skipping the Doublestar Path Collector test suite unintentionally for a long time. Just tried re-enabling those tests and they are all broken. I apologize for all the weird stuff I’ve introduced here trying to fix this without realizing what I did. I’m going to need to re-enable that test suite and then come up with a solution that actually works properly for this. Thank you everyone for your patience. Work has been busy, so I haven’t been able to spend any work time on this recently. This will probably need to wait until I have an evening available to clean all this stuff up.

Side note, yes I really should have a -version option and debug logging. I’m going to come up with a way to set those up as well. That is along the lines of the cleanup I’m trying to spend my time on for the next little while.

braydonk on Apr 11, 2023

I see, it seems the behavior changed when this diff was introduced in https://github.com/google/yamlfmt/pull/105/files#diff-64650b9066f9ed386546a3f7db1cc7c3a9bb4f422970d7506dd6b051b797f010

- match, err := doublestar.Match(filepath.Clean(pattern), path)
+ match, err := doublestar.PathMatch(filepath.Clean(pattern), absPath)

Basically data/k8s/charts/**/*.{yaml,yml} used to match the ( relative ) path data/k8s/charts/vendor/some-chart/templates/foo.yaml But now we convert the path into the absolute path /Users/ross/projects/core/data/k8s/charts/vendor/some-chart/templates/foo.yaml And the pattern does not match this absolute path

@rhbuf a quick fix for you would be to use this .yamlfmt instead

# yamlfmt mangles helm charts and throws errors, 
# `helm lint` should be used for linting charts
exclude:
- **/data/k8s/charts/**

@braydonk would you be able to clarify the need for an absolute path instead of the regular path ? Is an exclude pattern not starting with / or **/ valid ? I guess it would make sense to accept it and assume the pattern is relative to directory containing the .yamlfmt or the current directory if using the --exclude cli flag ?

badouralix on Apr 10, 2023

Thanks for the detailed explanation, @braydonk. It works.

esn89 on Mar 25, 2023