On Thu, Jul 12, 2018 at 2:21 PM Duncan Murdoch <murdoch.duncan at gmail.com> wrote:> I think I found the bug. The tools::checkRd function only processes > \Sexpr's with "stage=render". I think the author (who might have been > me, I forget) assumed that would imply all the earlier stages as well, > but apparently it doesn't. > > So you could use that as a workaround. > > I'll do some more checking, then submit a bug report and patch to Bugzilla. > > Duncan MurdochThanks much! I tried using stage=render, but then I get an error at install time: Warning: /private/var/folders/59/0gkmw1yj2w7bf2dfc3jznv5w0000gn/T/RtmpCG4Qz9/R.INSTALLec4743ba8cf4/ps/man/ps_handle.Rd:45-48: Section \Sexpr is unrecognized and will be dropped And indeed the whole section is dropped. Seems like there is no clean workaround here. Thanks again, G.
On Thu, Jul 12, 2018 at 2:30 PM G?bor Cs?rdi <csardi.gabor at gmail.com> wrote:> > On Thu, Jul 12, 2018 at 2:21 PM Duncan Murdoch <murdoch.duncan at gmail.com> wrote: > > I think I found the bug. The tools::checkRd function only processes > > \Sexpr's with "stage=render". I think the author (who might have been > > me, I forget) assumed that would imply all the earlier stages as well, > > but apparently it doesn't. > > > > So you could use that as a workaround. > > > > I'll do some more checking, then submit a bug report and patch to Bugzilla. > > > > Duncan Murdoch > > Thanks much! I tried using stage=render, but then I get an error at > install time: > > Warning: /private/var/folders/59/0gkmw1yj2w7bf2dfc3jznv5w0000gn/T/RtmpCG4Qz9/R.INSTALLec4743ba8cf4/ps/man/ps_handle.Rd:45-48: > Section \Sexpr is unrecognized and will be dropped > > And indeed the whole section is dropped. > > Seems like there is no clean workaround here. > > Thanks again, > G.Btw. would it make sense to just allow \Sexpr as a top level section? Maybe here: https://github.com/wch/r-source/blob/98e9999eb0e8616550632a1675e4d2dbe630d5e4/src/library/tools/R/RdConv2.R#L500-L503 At least if stage=render, there is no way to check if the returned value is always a valid top level section, anyway. If it is not a valid section (or \Sexpr returns some bad markup in general), then the user gets a render-time error, but with stage=render I guess one cannot do better. G.
On 12/07/2018 9:46 AM, G?bor Cs?rdi wrote:> On Thu, Jul 12, 2018 at 2:30 PM G?bor Cs?rdi <csardi.gabor at gmail.com> wrote: >> >> On Thu, Jul 12, 2018 at 2:21 PM Duncan Murdoch <murdoch.duncan at gmail.com> wrote: >>> I think I found the bug. The tools::checkRd function only processes >>> \Sexpr's with "stage=render". I think the author (who might have been >>> me, I forget) assumed that would imply all the earlier stages as well, >>> but apparently it doesn't. >>> >>> So you could use that as a workaround. >>> >>> I'll do some more checking, then submit a bug report and patch to Bugzilla. >>> >>> Duncan Murdoch >> >> Thanks much! I tried using stage=render, but then I get an error at >> install time: >> >> Warning: /private/var/folders/59/0gkmw1yj2w7bf2dfc3jznv5w0000gn/T/RtmpCG4Qz9/R.INSTALLec4743ba8cf4/ps/man/ps_handle.Rd:45-48: >> Section \Sexpr is unrecognized and will be dropped >> >> And indeed the whole section is dropped. >> >> Seems like there is no clean workaround here. >> >> Thanks again, >> G. > > Btw. would it make sense to just allow \Sexpr as a top level section? > Maybe here: > https://github.com/wch/r-source/blob/98e9999eb0e8616550632a1675e4d2dbe630d5e4/src/library/tools/R/RdConv2.R#L500-L503I think the mental model is that \Sexpr is essentially a preprocessor directive, and should disappear before rendering. It's the final result that needs to be checked. So \Sexpr should be able to produce top level sections, but it shouldn't be considered to be one. The reason checkRd has a stages argument is because it might be working on partially processed Rd objects, rather than the original .Rd file. Each of the stages should be processed exactly once, in order.> > At least if stage=render, there is no way to check if the returned > value is always a valid top level section, anyway.But you can check if it is sometimes, which is better than nothing. Most \Sexpr content is really simple, so this is probably good enough. Duncan> If it is not a valid section (or \Sexpr returns some bad markup in > general), then the user gets a render-time error, > but with stage=render I guess one cannot do better. > > G. >
On 12/07/2018 9:46 AM, G?bor Cs?rdi wrote:> On Thu, Jul 12, 2018 at 2:30 PM G?bor Cs?rdi <csardi.gabor at gmail.com> wrote: >> >> On Thu, Jul 12, 2018 at 2:21 PM Duncan Murdoch <murdoch.duncan at gmail.com> wrote: >>> I think I found the bug. The tools::checkRd function only processes >>> \Sexpr's with "stage=render". I think the author (who might have been >>> me, I forget) assumed that would imply all the earlier stages as well, >>> but apparently it doesn't. >>> >>> So you could use that as a workaround. >>> >>> I'll do some more checking, then submit a bug report and patch to Bugzilla. >>> >>> Duncan Murdoch >> >> Thanks much! I tried using stage=render, but then I get an error at >> install time: >> >> Warning: /private/var/folders/59/0gkmw1yj2w7bf2dfc3jznv5w0000gn/T/RtmpCG4Qz9/R.INSTALLec4743ba8cf4/ps/man/ps_handle.Rd:45-48: >> Section \Sexpr is unrecognized and will be dropped >> >> And indeed the whole section is dropped. >> >> Seems like there is no clean workaround here. >> >> Thanks again, >> G. > > Btw. would it make sense to just allow \Sexpr as a top level section? > Maybe here: > https://github.com/wch/r-source/blob/98e9999eb0e8616550632a1675e4d2dbe630d5e4/src/library/tools/R/RdConv2.R#L500-L503 > > At least if stage=render, there is no way to check if the returned > value is always a valid top level section, anyway. > If it is not a valid section (or \Sexpr returns some bad markup in > general), then the user gets a render-time error, > but with stage=render I guess one cannot do better. >I believe the problem is in the tools:::.check_package_parseRd function. It checks the .Rd files rather than parsed Rd objects, so it needs to be told to expand all \Sexpr's, not just render-time ones. I think it is fixed by the below patch to R-devel. Can you give it a try before I submit the bug report? Duncan Murdoch Index: src/library/tools/R/QC.R ==================================================================--- src/library/tools/R/QC.R (revision 74954) +++ src/library/tools/R/QC.R (working copy) @@ -6430,7 +6430,8 @@ if(basename(f) %in% c("iconv.Rd", "showNonASCII.Rd")) def_enc <- TRUE tmp <- tryCatch(suppressMessages(checkRd(f, encoding = enc, def_enc = def_enc, - macros = macros)), + macros = macros, + stages = c("build", "install", "render"))), error = identity) if(inherits(tmp, "error")) { bad <- c(bad, f)