ikatyang / tree-sitter-yaml

YAML grammar for tree-sitter
https://ikatyang.github.io/tree-sitter-yaml/
MIT License
94 stars 38 forks source link

Parsing is broken because of "multiline" array #55

Closed achojo closed 9 months ago

achojo commented 10 months ago

Selection_015

Defining array like in screenshot breaks parsing. I am almost sure this is a valid yaml. yq can parse it and I saw this kind of array definitions in other places.

char0n commented 10 months ago

@achojo I would say this is not valid YAML 1.2. Some parser implementation do support it, some don't.

If you look at https://yaml.org/spec/1.2.2/ and search for ] (empty space + ] char), you'll see that all the flow sequences have the c-sequence-end indicator at the same line as the last item within the flow sequence.

achojo commented 10 months ago

Yes, I see in specification there is no such example. But I have tried to test that syntax with different software and didn't find one that do not support that syntax. Here are some examples

Ansible

---
- name: Test
  hosts: localhost
  connection: local
  tasks: [
      {
          name: "Test task",
          ansible.builtin.debug: {
              msg: "test"
          }
      },
      {
          name: "Test task2",
          ansible.builtin.debug: {
              msg: "test2"
          }
      }
  ]
  vars:
      testvar: value

Docker compose

---
version: "3.9"

services:
  db:
    image: nginx
    ports: [
      "8443:443",
      "8080:80"
    ]

Kubernetes

---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx-deployment
spec:
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers: [
        {
          name: "nginx",
          image: "nginx:1.14.2",
          ports: [
            {
              containerPort: 80
            }
          ]
        }
      ]

Alacritty

key_bindings: [
  {key: V, mods: Control|Shift, action: Paste},
  {key: C, mods: Control|Shift, action: Copy},
  {key: Insert, mods: Shift, action: PasteSelection},
  {key: Key0, mods: Control, action: ResetFontSize},
  {key: Equals, mods: Control, action: IncreaseFontSize},
  {key: Plus, mods: Control, action: IncreaseFontSize},
  {key: Minus, mods: Control, action: DecreaseFontSize},
  {key: F11, mods: None, action: ToggleFullscreen},
  {key: Paste, mods: None, action: Paste},
  {key: Copy, mods: None, action: Copy},
  {key: L, mods: Control, action: ClearLogNotice},
  {key: L, mods: Control, chars: "\x0c"},
  {key: PageUp, mods: Shift, action: ScrollPageUp, mode: ~Alt},
  {key: PageDown, mods: Shift, action: ScrollPageDown, mode: ~Alt},
  {key: Home, mods: Shift, action: ScrollToTop, mode: ~Alt},
  {key: End, mods: Shift, action: ScrollToBottom, mode: ~Alt}
]

from efm-langserver config

    root-markers: [
      ".yamllint"
    ]

from kubie config

    exclude: [
        ~/.kube/kubie.yaml
    ]

yamllint and github

Listed programs all parsed provided configs without problem. Also text editors vim, vscode, kate also parsed without problems.

But if it is still not convincing and the goal of this parser just to meet requirements of specification then I think this issue can be closed.

char0n commented 9 months ago

Listed programs all parsed provided configs without problem. Also text editors vim, vscode, kate also parsed without problems.

Yes, I'd agree, most parses do support this invalid syntax for practical reasons

But if it is still not convincing and the goal of this parser just to meet requirements of specification then I think this issue can be closed.

Well, this is the only YAML 1.2 grammar for the tree-sitter. We're stuck with it...and it also seems that author has abandoned this grammar as well.

achojo commented 9 months ago

Ok, I am closing this issue.