Skip to content

Commit

Permalink
fixes #752: now we treat the closing mark (e.g. @ or ```) as part of …
Browse files Browse the repository at this point in the history
…the code chunk, instead of the beginning of a text chunk

if you use version control tools, you will see a couple of blank lines removed

this is actually a problem from day one once asked by @dmbates: https://groups.google.com/d/msg/knitr/Pf8yx7My01s/upl3QiqE2xcJ
  • Loading branch information
yihui committed Apr 13, 2014
1 parent 4d9539b commit 2b52194
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 2 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@

- the default value for the chunk option `tidy` is `FALSE` now, which means the R source code in chunks will no longer be reformatted by `formatR::tidy.source()` by default; this feature must be explicitly turned on by `tidy=TRUE`, and it has brought a lot of confusion in the past, so it is perhaps a good idea not to reformat the source code by default

- now we treat the closing mark (e.g. `@` in Sweave or the three backticks in R Markdown) as part of the code chunk, instead of the beginning of a text chunk, and a consequence for this change is that **knitr** no longer adds blank lines to the beginning of the text chunks (thanks, Thell 'Bo' Fowler, #752)

- inline R expressions will no longer be evaluated in `try()`, which means errors in inline R code will be emitted immediately

- the first argument of the `plot` hook is the filename of the plot now; in previous versions, it was a vector of length 2 (basename and file extension); see `?hook_plot`
Expand Down
10 changes: 8 additions & 2 deletions R/parser.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ split_file = function(lines, set.preamble = TRUE, patterns = knit_patterns$get()

blks = grepl(chunk.begin, lines)
txts = filter_chunk_end(blks, grepl(chunk.end, lines))
tmp = logical(n); tmp[blks | txts] = TRUE; lines[txts] = ''
# tmp marks the starting lines of a code/text chunk by TRUE
tmp = blks | head(c(TRUE, txts), -1)

groups = unname(split(lines, cumsum(tmp)))
if (set.preamble)
Expand Down Expand Up @@ -45,8 +46,12 @@ dep_list = new_defaults()

## separate params and R code in code chunks
parse_block = function(input, patterns) {
n = length(input)
# remove the optional chunk footer
if (n >=2 && grepl(patterns$chunk.end, input[n])) input = input[-n]

block = strip_block(input, patterns$chunk.code)
n = length(block); chunk.begin = patterns$chunk.begin
chunk.begin = patterns$chunk.begin
params.src = if (group_pattern(chunk.begin)) {
str_trim(gsub(chunk.begin, '\\1', block[1]))
} else ''
Expand All @@ -57,6 +62,7 @@ parse_block = function(input, patterns) {
}

label = params$label; .knitEnv$labels = c(.knitEnv$labels, label)
# remove the chunk header
code = block[-1L]
if (length(code)) {
if (label %in% names(knit_code$get())) stop("duplicate label '", label, "'")
Expand Down
13 changes: 13 additions & 0 deletions tests/testit/test-parser.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,19 @@ assert(
alist(label='abc-function', fig.path="foo/bar-"))
)

res = split_file(
c('abc', '```{r foo}', '1+1', '```{r bar}', '2+2', '```', 'def'),
patterns = all_patterns$md
)
assert(
'split_file() treats ``` as part of code chunk instead of beginning of text chunk',
# the foo chunk does not have a closing mark
identical(knit_code$get('foo'), '1+1'),
identical(knit_code$get('bar'), '2+2'),
# before knitr v1.6, the text chunk was c('', 'def')
identical(res[[4]][['input']], 'def')
)
knit_code$restore(); knit_concord$restore()

res = parse_inline(c('aaa \\Sexpr{x}', 'bbb \\Sexpr{NA} and \\Sexpr{1+2}',
'another expression \\Sexpr{rnorm(10)}'), all_patterns$rnw)
Expand Down

5 comments on commit 2b52194

@e-pet
Copy link

@e-pet e-pet commented on 2b52194 Apr 1, 2015

Choose a reason for hiding this comment

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

This does indeed prevent the whitespace from occuring, but it does not prevent the paragraph from breaking. It would be nice to have an option to do that, otherwise one still needs to do \noindent.

@yihui
Copy link
Owner Author

@yihui yihui commented on 2b52194 Apr 1, 2015

Choose a reason for hiding this comment

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

I guess you can do knit_hooks$set(text = function(x) paste('\\noindent', x)).

@e-pet
Copy link

@e-pet e-pet commented on 2b52194 Apr 1, 2015

Choose a reason for hiding this comment

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

Yes I can (thanks!), but it's kind of hackish ;) As I said, it would be nice to have a general solution to this problem that seems rather common to me. Also note that your solution does not work in all cases, consider e.g. situations where there's a parskip between paragraphs. See, e.g., this question on SO.

@yihui
Copy link
Owner Author

@yihui yihui commented on 2b52194 Apr 1, 2015

Choose a reason for hiding this comment

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

Oh I love LaTeX and typesetting... :) I'm afraid I will have to count on you to find a general solution, since I'm unlikely to have the bandwidth to deal with this issue in the near future.

@e-pet
Copy link

@e-pet e-pet commented on 2b52194 Apr 1, 2015

Choose a reason for hiding this comment

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

The endless wonders of Latex... ;) Right now I also don't have the time, but maybe in a month or so I'll have some spare energy. If yes, I'll give it a try! In the meantime, thank you for your wonderful tool and the quick responses! :-)

Please sign in to comment.