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

Feature Request: Add rule for colons similar to yamllint #92

Closed
myii opened this issue Nov 2, 2019 · 5 comments
Closed

Feature Request: Add rule for colons similar to yamllint #92

myii opened this issue Nov 2, 2019 · 5 comments
Labels

Comments

@myii
Copy link
Contributor

myii commented Nov 2, 2019

Taking this example from the epel-formula:

    'common':{
  • While this is from map.jinja, something similar could appear in an .sls file.
  • The issue here is that there isn't a space after the colon.

Here's the colons rule in yamllint:

While it may be too much to implement the whole rule, even just enforcing (at least) one space after colons should be sufficient.

@jbouter
Copy link
Member

jbouter commented Nov 2, 2019

I don't think we can just blindly assume there should always be a space after a colon though. It could appear in the contents of a file for example, and be valid. Then again, I guess that's true for most of the checks we perform.

Example:

example_file:
  file.managed:
    - name: /etc/example.txt
    - contents: |
        line:with:colons:without:spaces

@myii
Copy link
Contributor Author

myii commented Nov 3, 2019

@jbouter Sure, not after every colon! That's where the linter needs to do it's job to identify the specific places.

@dawidmalina
Copy link

It would be very good if we could include all yamllint rules - not only colons.

@dawidmalina
Copy link

Proposed test cases:

# the following code snippet would fail:
/path/to/file1:
  file.managed:
    - contents : This is line 1

# the following code snippet would fail:
/path/to/file1:
    file.managed:
      - contents:[]

# the following code snippet would fail:
/path/to/file1:
    file.managed:
      - contents:{}

# the following code snippet would pass:
/path/to/file1:
  file.managed:
    - contents: This is line 1

# the following code snippet would pass:
/path/to/file1:
    file.managed:
      - contents: []

# the following code snippet would pass:
/path/to/file1:
    file.managed:
      - contents: {}

# the following code snippet would pass:
example_file:
  file.managed:
    - name: /etc/example.txt
    - contents: |
        line:with:colons:without:spaces

I will drop pull request this week.

@roaldnefs
Copy link
Member

Closing this issue in favour of #240.

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

No branches or pull requests

4 participants