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

start/end delineators for spin comments can be interchangeable #1801

Open
3 tasks done
dewittpe opened this issue Jan 24, 2020 · 0 comments · May be fixed by #1802
Open
3 tasks done

start/end delineators for spin comments can be interchangeable #1801

dewittpe opened this issue Jan 24, 2020 · 0 comments · May be fixed by #1802
Labels
feature Feature requests

Comments

@dewittpe
Copy link

It is posible for knitr::spin to use the starting and end delimiters for
comments backwards. This could be innocuous, but can also lead to problems.

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")
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.

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 in knitr::spin(text = txt3, knit = FALSE, format = "Rmd"): comments must be put in pairs of start and end delimiters

A second, related, issue with the check for comments, is that the current
error message

Error in knitr::spin(text = txt, knit = FALSE, format = “Rmd”) :
comments must be put in pairs of start and end delimiters

is good to know, but not helpful for finding the issues in the hair or text.

A possible solution would be to check the comments and force the regular
expression passed to comment in knir::spin to be the start and end
delimiters. I will be submitting a pull request shortly to address this
issue.

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)


By filing an issue to this repo, I promise that

  • I have fully read the issue guide at https://yihui.name/issue/.
  • I have provided the necessary information about my issue.
    • If I'm asking a question, I have already asked it on Stack Overflow or RStudio Community, waited for at least 24 hours, and included a link to my question there.
    • If I'm filing a bug report, I have included a minimal, self-contained, and reproducible example, and have also included xfun::session_info('knitr'). I have upgraded all my packages to their latest versions (e.g., R, RStudio, and R packages), and also tried the development version: remotes::install_github('yihui/knitr').
    • If I have posted the same issue elsewhere, I have also mentioned it in this issue.
  • I have learned the Github Markdown syntax, and formatted my issue correctly.

I understand that my issue may be closed if I don't fulfill my promises.

dewittpe added a commit to dewittpe/knitr that referenced this issue Jan 24, 2020
@dewittpe dewittpe linked a pull request Jan 24, 2020 that will close this issue
@cderv cderv linked a pull request Jan 29, 2021 that will close this issue
@cderv cderv added the feature Feature requests label Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants