Spin comments

[dewittpe]

These examples are the same as in #1801. With this PR an error is thrown
when delimiters are not as expected.

An innocuous example:

txt1 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "#' The check for start (/*) and end (*/) comments can fail to identify",
        "#' notable issues.  Consider a document with the first occurance of comment to",
        "#' be the *end* comment and the last comment to be an *start* comment.",
        "#'",
        "# */",
        "#' This text is in an 'comment' section but the characters denoting the comment",
        "#' are backwards. This will not be in the .Rmd file, even though the start and",
        "#' end delimiters are backwards",
        "# /*",
        "#'",
        "#' This doc can be spun to a .Rmd",
        sep = "\n")

x1 <- knitr::spin(text = txt1, knit = FALSE, format = "Rmd")
#> Error: comments must be put in pairs of start and end delimiters.
#>    * no starting delimiter; ended on line 7
#>   * started on line 11; no end delimiter

The miss ordering can result is something is difficult for a human. When
skimming txt2 I would ex the first “comment” block to be in the .Rmd file,
the R code chunk to be omitted, the statment “annother comment” to be in the
.Rmd, and last line to be omitted. This is assuming that an error isn’t
thrown because of the first comment delimiters is the “end” delimiters and
the last delimiter is the “start” delimiter.

txt2 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "# */",
        "#' This text is in an 'comment' section but the characters denoting the comment",
        "#' are backwards. This will not be in the .Rmd file.",
        "# /*",
        "#'",
        "#' A little R code",
        "2 + 2",
        "#'",
        "# */",
        "#' Another comment",
        "# /*",
        "#' This doc can be spun to a .Rmd, but the comment start and end delimators are",
        "#' interchangeable.",
        sep = "\n")

x2 <- knitr::spin(text = txt2, knit = FALSE, format = "Rmd")
#> Error: comments must be put in pairs of start and end delimiters.
#>    * no starting delimiter; ended on line 3
#>   * started on line 6; ended on line 11
#>   * started on line 13; no end delimiter

What about nested comments? If the comment delimiters were correctly
specified the following would work, but as written, an error is thrown.

txt3 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "#' text ...",
        "#'",
        "# */ outside comment",
        "#",
        "# ... code that is commented out during dev work, perhaps,",
        "#",
        "# */",
        "# A comment that is a comment and will be here for the long term.",
        "# /*",
        "#",
        "# ... code and text, next line ends outside comment.",
        "# /*",
        "#'",
        "#' document text",
        sep = "\n")

x3 <- knitr::spin(text = txt3, knit = FALSE, format = "Rmd")
#> Error: comments must be put in pairs of start and end delimiters.
#>    * no starting delimiter; ended on line 9
#>   * started on line 11; no end delimiter
#>   * started on line 14; no end delimiter

With correct comments:

txt1 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "#' The check for start (/*) and end (*/) comments can fail to identify",
        "#' notable issues.  Consider a document with the first occurance of comment to",
        "#' be the *end* comment and the last comment to be an *start* comment.",
        "#'",
        "# /*",
        "#' This text is in an 'comment' section but the characters denoting the comment",
        "#' are backwards. This will not be in the .Rmd file, even though the start and",
        "#' end delimiters are backwards",
        "# */",
        "#'",
        "#' This doc can be spun to a .Rmd",
        sep = "\n")

x1 <- knitr::spin(text = txt1, knit = FALSE, format = "Rmd")
cat(x1, sep = "\n")
#> # A reprex for knitr::spin comments
#> 
#> The check for start (/*) and end (*/) comments can fail to identify
#> notable issues.  Consider a document with the first occurance of comment to
#> be the *end* comment and the last comment to be an *start* comment.
#> 
#> 
#> This doc can be spun to a .Rmd

The miss ordering can result is something is difficult for a human. When
skimming txt2 I would ex the first “comment” block to be in the .Rmd file,
the R code chunk to be omitted, the statment “annother comment” to be in the
.Rmd, and last line to be omitted. This is assuming that an error isn’t
thrown because of the first comment delimiters is the “end” delimiters and
the last delimiter is the “start” delimiter.

txt2 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "# /*",
        "#' This text is in an 'comment' section but the characters denoting the comment",
        "#' are backwards. This will not be in the .Rmd file.",
        "# */",
        "#'",
        "#' A little R code",
        "2 + 2",
        "#'",
        "# /*",
        "#' Another comment",
        "# */",
        "#' This doc can be spun to a .Rmd, but the comment start and end delimators are",
        "#' interchangeable.",
        sep = "\n")

x2 <- knitr::spin(text = txt2, knit = FALSE, format = "Rmd")
cat(x2, sep = "\n")
#> # A reprex for knitr::spin comments
#> 
#> 
#> A little R code
#> 
#> ```{r }
#> 2 + 2
#> ```
#> 
#> 
#> This doc can be spun to a .Rmd, but the comment start and end delimators are
#> interchangeable.


txt3 <-
  paste("#' # A reprex for knitr::spin comments",
        "#'",
        "#' text ...",
        "#'",
        "# /* outside comment",
        "#",
        "# ... code that is commented out during dev work, perhaps,",
        "#",
        "# /*",
        "# A comment that is a comment and will be here for the long term.",
        "# */",
        "#",
        "# ... code and text, next line ends outside comment.",
        "# */",
        "#'",
        "#' document text",
        sep = "\n")

x3 <- knitr::spin(text = txt3, knit = FALSE, format = "Rmd")
cat(x3, sep = "\n")
#> # A reprex for knitr::spin comments
#> 
#> text ...
#> 
#> 
#> document text

xfun::session_info("knitr")
#> R version 3.6.2 (2019-12-12)
#> Platform: x86_64-apple-darwin15.6.0 (64-bit)
#> Running under: macOS Catalina 10.15.2
#> 
#> Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8
#> 
#> Package version:
#>   evaluate_0.14   glue_1.3.1      graphics_3.6.2  grDevices_3.6.2
#>   highr_0.8       knitr_1.27.2    magrittr_1.5    markdown_1.1   
#>   methods_3.6.2   mime_0.8        stats_3.6.2     stringi_1.4.5  
#>   stringr_1.4.0   tools_3.6.2     utils_3.6.2     xfun_0.12      
#>   yaml_2.2.0

^{Created on 2020-01-24 by the reprex package (v0.3.0)}

[CLAassistant]

All committers have signed the CLA.

[yihui]

Thanks for this, @dewittpe, and sorry for the long delay.

The underlying bug is real: spin() only compared the counts of start (# /*) and end (# */) delimiters, so mis-ordered but equal-count delimiters would pass the check and silently drop the lines in between. I've picked this up, rebased it onto the current master, and pushed a follow-up commit to your branch:

• Rewrote check_comments() in knitr's code style — it now walks the delimiters in line order and matches each start with the next end, instead of grepl()-ing over synthesized element names. It reports each unmatched delimiter with its line number via stop2().
• Added spin() tests covering correctly-paired removal and the mis-ordered / unmatched error cases.
• Moved the NEWS entry to the current version and expanded it.

Your original commits are preserved; my changes sit on top. It's now mergeable. Thanks again for the reprexes in the PR description — they made the failure modes easy to verify.