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

Allow highlighting of some empty expressions #1310

Merged
merged 2 commits into from
May 20, 2020

Conversation

wkmor1
Copy link
Contributor

@wkmor1 wkmor1 commented Apr 24, 2020

Improved version of #1307 accounting for problems caused by empty expressions consisting of empty strings or whitespace only.

Prevents highlight markup from being stripped from code that consists entirely of code comments (only lines beginning with #'s).

A partial fix for #1296, but would still not syntax highlight code chunks that resulted in syntax
errors (e.g., when knitr chunk options collapse = TRUE and comment = NA are set)

Improved version of r-lib#1307 accounting for problems caused by empty expressions consisting of empty
strings or whitespace only.

Prevents highlight markup from being stripped from code that consists entirely of code comments (only
lines beginning with #'s).

A partial fix for r-lib#1296, but would still not syntax
highlight code chunks that resulted in syntax
errors (e.g., when knitr chunk options collapse =
TRUE and comment = NA are set)
@wkmor1
Copy link
Contributor Author

wkmor1 commented Apr 24, 2020

@jayhesselberth pretty sure this fixes the site build issue. Narrowed the problem down to this blank line

which sent the white-space only expression to pkgdown::highlight_text causing the error in highlight::highlight

R/highlight.r Outdated
# Failed to parse, or yielded empty expression
if (length(expr) == 0) {
# Failed to parse or is white-space only/empty
if (is.null(expr) || !nchar(trimws(text))) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just test for the empty string here: trimws(text) == ""

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I was also aiming to catch cases where text = character(0) but now that I look at it !nchar wouldn't do that anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I don't even need to do that cos it is covered at

stopifnot(is.character(text), length(text) == 1)
My bad.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 3936880

@jayhesselberth jayhesselberth merged commit c1db398 into r-lib:master May 20, 2020
@jayhesselberth
Copy link
Collaborator

Thanks!

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.

None yet

2 participants