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

Fix cli evaluating brace contents in error msgs #2524

Merged
merged 2 commits into from
May 9, 2024

Conversation

zeehio
Copy link
Contributor

@zeehio zeehio commented May 9, 2024

The cli functions evaluate things in braces.

Errors may include braces that we do not want to evaluate

For instance, pkgdown was reporting an error when building my vignettes:

Error in `map()`:
i In index: 1.
Caused by error:
! Could not evaluate cli `{}` expression: `document`.
Caused by error:
! object 'document' not found

Because the rendering of my vignette reported:

LaTeX Error: Missing \begin{document}.

and cli_abort intepreted {document} as an
object to be evaluated.

Now it reports:

Error in `build_article()`:
! Failed to render vignettes/...
x ! LaTeX Error: Missing \begin{document}.
...

as expected

The cli functions evaluate things in braces.

Errors may include braces that we do not want to evaluate

For instance, pkgdown was reporting an error:

```
Error in `map()`:
i In index: 1.
Caused by error:
! Could not evaluate cli `{}` expression: `document`.
Caused by error:
! object 'document' not found
```

Because the rendering of my vignette reported:

```
LaTeX Error: Missing \begin{document}.
```

and `cli_abort` intepreted `{document}` as an
object to be evaluated.

Now it reports:

```
Error in `build_article()`:
! Failed to render vignettes/...
x ! LaTeX Error: Missing \begin{document}.
...
```

as expected
R/rmarkdown.R Outdated
@@ -37,6 +37,9 @@ render_rmarkdown <- function(pkg, input, output, ..., seed = NULL, copy_images =
callr::r_safe(rmarkdown_render_with_seed, args = args, show = !quiet),
error = function(cnd) {
lines <- strsplit(gsub("^\r?\n", "", cnd$stderr), "\r?\n")[[1]]
lines <- lines[nchar(lines) >0]
lines <- gsub("{", "{{", lines, fixed = TRUE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind pulling this out into an escape_cli() function? I think that makes it slightly easier to understand the purpose of the code, and I suspect we'll need to use it elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Thanks for your quick review, @hadley!

@hadley hadley merged commit 58b61da into r-lib:main May 9, 2024
12 checks passed
@hadley
Copy link
Member

hadley commented May 9, 2024

Thanks for noticing this problem and figuring out a fix!

@zeehio zeehio deleted the fix-error-report branch May 9, 2024 23:06
SebKrantz pushed a commit to SebKrantz/pkgdown that referenced this pull request Jun 1, 2024
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.

2 participants