-
Notifications
You must be signed in to change notification settings - Fork 331
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
Unclear error message when using code chunk options with yaml and an extra space is present #4306
Comments
This is indeed unfortunate, thanks for reporting. We already do some patching of the error messages to point to the correct location, but it seems that we're not catching this one. |
Oh, I understand now. Ok. This is an annoying one. This is actually coming from knitr, because it's an Consider this instead:
That document generates this error message:
Which still has imprecise squiggles, but is at least pointing to the correct place in the document. Then, consider this:
That yields this error:
So, the problem only happens when you use the wrong comment syntax (which means quarto doesn't detect it as a yaml block to validate), and send it to knitr (which seems to ignore the wrong comment syntax and then fail to parse the yaml correctly, but give error messages with bad error numbers). The problem is that knitr doesn't track line numbering precisely enough, and the error that comes back from knitr is too ambiguous for us to correct it after the fact. Ultimately, I believe this needs to be fixed on knitr (@cderv). |
I would say that the problem is that knitr will use the same logic to recognized comment for a language and so using However, yes knitr will also support the This means both
and ```{sql}
#| label: test multiqueries -5
#| connection: db
select 1
``` are valid for knitr and in R Markdown. However, regarding Quarto,
Before talking about solution, @cscheid do you think there is other side effect that supporting One thing we could do is being more strict for Quarto document and not checking the fallback. Easy for us to do.
I don't think we should try to report an error similar to the Quarto YAML validation but in fact be sure it is triggered as I suggest above.
I agree - I am not sure it is a good idea for Quarto to adapt to knitr behavior in R document as this would make a difference with other computation engine. |
I discussed this with @jjallaire a long time ago and the conclusion is that we want strongly for the code syntax in code blocks to be valid code in those languages. If you support (eg) |
Yes I agree with that. We don't do that by default in knitr. So I think we could deactivate support for fallback at least when knitr is used for Quarto document. However, it will not help find the error because the options won't be parsed at all. This is what I get by doing the above (using this branch yihui/knitr#2225) rendering with Quarto ---
title: "Example"
format: html
engine: knitr
---
```{r}
# install.packages(c("DBI", "RSQLite"))
db = DBI::dbConnect(RSQLite::SQLite(), ":memory:")
```
```{sql}
#| label: test multiqueries -5
#| connection: db
select 1
```
I decided to suggest something with an error as otherwise the code chunk will be executed without chunk options being parsed (see PR comment). I believe that it why we support the fallback in the first place, but this won't "teach" users to use the correct comment line. Do you think that would be better ? @coatless do you find that more useful ? |
@cderv I think the error message definitely needs to be improved to clarify the source of the real error (telling users that "line 2, column 12" doesn't mean the line/column in the whole source document but in the YAML part of a code chunk).
I also had a discussion with JJ a long time ago but I don't think we convinced each other. I prefer using Supporting BTW, in our original discussion, I also preferred the traditional CSV-like syntax for chunk options over the YAML syntax because sometimes YAML is hard to please. It's notoriously picky about spaces (fortunately, our YAML hero @cscheid came to rescue with a whole set of tools). That's why knitr also supports both syntaxes. That said, I have no problem with us promoting the YAML syntax as well as the language-specific comment-hash. I agree that we'd better educate users with only one way to do things. Anyway, no matter we support |
Yes you're right - I agree with that. I opened an issue to track this in knitr as an independent issue |
First, thank you everyone for looking in-depth at the issue. I've aimed to reply to the comments with quoted replies:
@cderv yes, this would be far more useful. It provides course staff the ability to more easily understand where to look. It also helps students out as they have a better error message to google. Only one other question: Would it be possible to identify the code chunk by a label or the code language type at this point? Or is that not possible either? (Thinking about a polygot document)
@yihui I think the use of Even the first description of the different engines omits a description on language-specific comment usage: https://quarto.org/docs/visual-editor/technical.html#embedded-code |
Here is the error message that I proposed: yihui/knitr#2226 (comment) It can contain the line numbers of the source document indicating where the error comes from. |
This is the error we get now when using knitr v1.45
|
Bug description
This issue came up earlier today when a student was having problems identifying why their quarto document wasn't able to be rendered.
In particular, the student accidentally spaced the code chunk option one extra, that is:
Thus, when the Quarto document was rendered, we were seeing:
This is a misleading error as it implies that the error falls inside of the document customization header. Instead, the error was related to a code chunk option.
Would it be possible to provide more localized support for bad code chunk specification?
my-homework.qmd
:Version information:
Checklist
The text was updated successfully, but these errors were encountered: