imbal / safeyaml

SafeYAML: A linter for YAML-favoured JSON (& autoformatting too!)
240 stars 11 forks source link

Nested Keys #4

Open tef opened 6 years ago

tef commented 6 years ago
- name: value
- name: value

Is very popular

marksutherland commented 6 years ago

Yup, kubernetes manifest files seem to love them, for example. Here's a simple case and the output it causes:

parent:
  - child: value
    another: please
➭ ./safeyaml.py --fix example.txt
['example.txt']:2:5:The parser is trying its very best but could only make out 'child', but there is other junk on that line. You fix it.
aanand commented 6 years ago

We have 3 options in the face of any particular practice:

  1. Reject outright
  2. Fix when --fix is passed
  3. Accept

The problem with a map inside a list starting on the same line is that, in the one-element case, it's very easy to confuse with a string. We had this exact problem in Compose, where people would put a space in their volume mount:

web:
  build: web
  volumes:
    - web:/web

chat:
  build: chat
  volumes:
    - chat: /chat

Above, web:/web is a string, and chat: /chat is a map. A linter therefore can't make a safe call as to the user's intention.

Supposing we went with 2, the output of running safeyaml --fix on the above would be:

web:
  build: "web"
  volumes:
    - "web:/web"

chat:
  build: chat
  volumes:
    -
      chat: "/chat"

Whereas if we went with 3, it would be:

web:
  build: "web"
  volumes:
    - "web:/web"

chat:
  build: "chat"
  volumes:
    - chat: "/chat"

On aesthetic grounds, I think I've argued myself around to option 3 - accept them. The rewritten version is equally clear in either case.

tef commented 6 years ago

There's a couple of things we can still do

https://github.com/imbal/safeyaml/blob/master/safeyaml.py#L26

I think we could allow indented lists to contain name:values and vice versa, with one layer of nesting.

i.e

foo: - 1
        - 2

or

-  foo: bar

but not

- foo: bar: baz
aanand commented 6 years ago

At the moment, bareword values can't have any YAML special characters inside.

Getting off topic but I do think this needs to change - there's lots of keys and values out there with colons in, for example, which safeyaml should be wrapping in quotes.

aanand commented 6 years ago

(expanded on the above in #10)

tef commented 6 years ago

See also

- -1
- -2
tef commented 6 years ago

The core of the indentation logic is in lines like this one

https://github.com/imbal/safeyaml/blob/e5ee1e0b02d1d69154f90729168f94e991ff17b3/safeyaml.py#L290

After reading a key and ': ', we scan ahead to the first non comment/whitespace token, and do different actions depending if it is on same or next line