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

fixes #252: Allow plain-code-blocks in RMarkdown #349

Merged
merged 3 commits into from Sep 18, 2018
Merged

fixes #252: Allow plain-code-blocks in RMarkdown #349

merged 3 commits into from Sep 18, 2018

Conversation

russHyde
Copy link
Collaborator

Added a test to the end of test.Rmd that included a plain-code-block like this:

This shouldn't cause a problem

That is, a code-block where the interpreter is not specified (these are allowable in Rmarkdown according to section 2.5.2 of Yihui's Rmarkdown book).

The code-block reproduced the Malformed File error described in issue #252

Plain-blocks match the knitr chunk.end pattern at both the start and the end
of the chunk. So when plain-blocks are present there was an extra 2k chunk ends
than starts.

A small function filter_chunk_end_positions was added to filter the positions
of the chunk-ends based on the chunk-starts and called from extract_r_source

Added a test to the end of test.Rmd that included a plain-code-block like this:

```
This shouldn't cause a problem
```

The code-block reproduced the `Malformed File` error described in #252

Plain-blocks match the knitr `chunk.end` pattern at both the start and the end
of the block. So when plain-blocks are present there was 2k _more_ chunk ends
than starts.

A small function `filter_chunk_end_positions` was added to filter the positions
of the chunk-ends based on the chunk-starts
@randy3k
Copy link
Collaborator

randy3k commented Sep 17, 2018

not the most beautiful way to find the blocks, but I beleive it would work.

@jimhester
Copy link
Member

Thanks! Could you add a

```{python}
```

example like in #322 as well to the Rmd.

Also please add a note to NEWS.md explaining the change and referencing the issue number and your GitHub username.

@russHyde
Copy link
Collaborator Author

Is it OK if I work on the python chunk bug in a separate PR? I'm fairly certain the changes included here don't touch that bug.

@randy3k - if you have a suggestion for a more idiomatic way to match the start/end positions I'm happy to modify the code; my first attempt wasn't even this beautiful ...

@russHyde
Copy link
Collaborator Author

Have updated NEWS.md. All the best

@russHyde
Copy link
Collaborator Author

For consistency with the other helper functions in extract.R, would you prefer that the function I added was placed after the main extract_r_source function in the source file?

@jimhester
Copy link
Member

would you prefer that the function I added was placed after the main extract_r_source function in the source file

Sure that makes sense, thanks!

Main function `extract_r_source` now precedes all it's helper functions (get_knitr_pattern, filter_chunk_end_positions, replace_prefix)
@jimhester
Copy link
Member

Thanks @russHyde for the PR and @randy3k for reviewing!

@jimhester jimhester merged commit fdd1950 into r-lib:master Sep 18, 2018
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

3 participants