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

try to add duplicate warning definition only for knitr #11875

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cderv
Copy link
Collaborator

@cderv cderv commented Jan 16, 2025

In knitr, warning: 'NA' is supported. How to allow that for auto completion ?

---
title: "test"
format: html
---

```{r}
#| warning: NA
warning("Test")
```

@cderv
Copy link
Collaborator Author

cderv commented Jan 16, 2025

@cscheid Do you know if our schema would allow to get a specific YAML validation for an option in cell
image

while this option is also used as document-execute.

The way I tried here, seems to have remove the execute part
image
while correctly added under cell
image

Normaly warning is also in execute
image

So do we have a way to have two warning definition with our schema ?

Thank you !

@cderv cderv force-pushed the knitr/schema-update branch from e543068 to c513898 Compare January 17, 2025 10:13
@cderv
Copy link
Collaborator Author

cderv commented Feb 20, 2025

A note that the document in example

---
title: "test"
format: html
---

```{r}
#| warning: NA
warning("Test")
```

Is not supposed to be rendered and should have a validation error at rendering time.

But this is not happening because of

const fullCell = mappedString(cell.sourceVerbatim, [{
start: lang.length + 6,
end: cell.sourceVerbatim.value.length - 3,
}]);

End line problem 😁 \r\n on Windows, so start should be + 7 not 6.

Fixing this will probably make a lot of document fails, at least those using valid knitr option but unknown to validation. We'll see.

I'll probably open a new PR to solve this issue.

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

Successfully merging this pull request may close these issues.

1 participant