More helpful error messages when Quarto chunk options are mis-specified

[cscheid]

Discussed in #2433

^{Originally posted by AmeliaMN September 13, 2022}
I posted this on the RStudio community as well, but figured out it should actually be here.

If you mess up when specifying your Quarto chunk options and use = instead of : in one of your code chunks (cough, former RMarkdown users), you will get a pretty impenetrable (to me) error about YAML. For example:

---
title: "Untitled"
format: pdf
editor: visual
---

```{r}
#| label: math
#| echo = false
2 * 2
```

Gives the error

processing file: test.qmd
Error in yaml::yaml.load(meta, handlers = list(expr = parse_only)) : 
  Scanner error: while scanning a simple key at line 2, column 1 could not find expected ':' at line 3, column 1
Calls: .main ... FUN -> parse_block -> partition_chunk -> <Anonymous>
Execution halted

I spent way too much time looking for errors in my header YAML, forgetting that those #| comments are basically YAML now, too. One of the problems is the line 3 part. I was looking in the third line of my Quarto document!

Not sure if it would be possible, but it would be awesome to have an error message that said something like Scanner error: while scanning the options in chunk 1, ...

[cscheid]

Woooo boy, this is a fun one.

We're supposed to check against these echo = FALSE mistakes explicitly, because we agree that this is a common error and the message is confusing. We have written custom code for this! And also, how is that invalid YAML even making it to knitr? Why aren't we catching this?!

Oh. Ohhh. Tree-sitter.

A few months ago, we made the change to parse all of our yaml with tree-sitter (including in the CLI), so that we would have uniformity between the parses generated in the IDEs and the ones in the CLI.

I used to think that was a good idea, but I didn't realize one of the side effects. But now we are more tolerant of bad parses in our YAML chunks. So tree-sitter is happy to report that

label: math
echo = false

is a valid YAML object. But of course it's not, as a quick trip to http://www.yamllint.com/ will show you. In addition, it says that our YAML object is, in JSON notation, { "label": "math", "echo = false": null }. Then, when we turn to YAML validation, of course this passes, because we have nothing to say about a echo = false key 🤦.

Worse yet, it doesn't look that the tree-sitter API can actually distinguish between a partial parse that emitted something but with an error, and a full parse that emitted something that is actually a valid parse.

not a solution: drop tree-sitter

We use tree-sitter in interactive mode in the IDEs, and currently we have a single codebase to parse code cells, both in the IDE and not. If we drop tree-sitter, then we lose error tolerance during completions as users type. That doesn't work

maybe a solution: disallow spaces in mapping keys

We could say that echo = false will never be a valid key in our YAML. I'm pretty sure we won't want to ever do it, but:

$ deno
> import * as mod from "https://deno.land/std@0.155.0/encoding/yaml.ts";
> mod.parse('"echo = false": "weird"')
{ "echo = false": "weird" }

It turns out that yaml does allow spaces in its keys, if the keys are quoted. So this is valid YAML:

label: math
"echo = false": null

and has the same JSON representation as the invalid one from tree-sitter. So if we disallow spaces in keys, we fundamentally disallow some (deeply weird) yaml that someone might want to eventually write. I think this is my least bad idea to fix the problem.

[cscheid]

Ok, new error message:

$ quarto render 2434.qmd
ERROR: Validation of YAML cell metadata failed.
(line 9, columns 4--16) property name echo = false is invalid

✖ The value echo = false is a string.
ℹ In YAML, key-value pairs in objects must be separated by a colon and a space.
ℹ Did you mean echo: false instead?

ERROR: Render failed due to invalid YAML.