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

Dynamic examples, take two #962

Closed
gaborcsardi opened this issue Nov 12, 2019 · 12 comments · Fixed by #993
Closed

Dynamic examples, take two #962

gaborcsardi opened this issue Nov 12, 2019 · 12 comments · Fixed by #993
Labels
feature a feature request or enhancement

Comments

@gaborcsardi
Copy link
Member

PoC implementation is here: https://github.com/gaborcsardi/dynex#readme

  • Uses \dontshow{} tags to hide the check for the condition.
  • No more \Sexpr{} tags, so don't need to install the package for R CMD build.
  • Multiple @dynex tags work fine.

To include this in roxygen2, we would need to just copy over the code, probably rename the tag to @examplesIf, and add support for this in R6 classes.

@hadley
Copy link
Member

hadley commented Nov 12, 2019

Just to be clear, I'm imagining that

#' @dynex
#' interactive()
#' name <- readline("name> ")
#' cat("hello", name)

would become

#' @examplesIf interactive()
#' name <- readline("name> ")
#' cat("hello", name)

@hadley
Copy link
Member

hadley commented Nov 12, 2019

Since the release is delayed and you've already implemented this, maybe it's worth trying to get into 7.0.0?

@gaborcsardi
Copy link
Member Author

I think it would be better to experiment with this a bit, I already found an issue with the printing, only the last expression within the if ... { is printed, so we need break apart the expression or maybe rather use withAutoprint().

It is pretty easy to add the \dontshow{} to the @examples manually as well, I think we can try that first and see how it goes.

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Nov 12, 2019

Just to be clear, I'm imagining that ... would become

Right, it does not matter which line you put the condition, roxygen2 parses them the same way, and yeah, we would call it @exampleIf.

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Nov 12, 2019

This seems good, except that withAutoprint() needs R 3.4.x.

#' @examples
#' \dontshow{ if (pingr::is_online()) withAutoprint(\{ }
#' ...
#' \dontshow{ \}) }

@hadley
Copy link
Member

hadley commented Nov 20, 2019

withAutoprint() looks like a pretty thin wrapper around source(). But I do wonder using it will make it harder in pkgdown?

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Nov 20, 2019

FWIW I started using this approach in pkgsearch, but in the end had to call an internal helper function, to make it work on older R:
https://github.com/r-hub/pkgsearch/blob/26c4cc24b9296135b6238adc7631bc5250509486/R/api.R#L51
https://github.com/r-hub/pkgsearch/blob/26c4cc24b9296135b6238adc7631bc5250509486/R/exif.R#L2

pkgdown looks good to me, somehow auto-magically:
https://r-hub.github.io/pkgsearch/reference/pkg_search.html

withAutoprint() is indeed a small source() wrapper, but the source() arguments and features it uses were also introduced together with withAutoprint() so using source() directly does not help us here.

@hadley
Copy link
Member

hadley commented Nov 20, 2019

pkgdown looks great! I wonder where we can put the helper so any package can easily access it.

@gaborcsardi
Copy link
Member Author

I guess we could put the code in the package itself. Maybe that code can be shortened somehow, and then we can just spell it out in the Rd file.

@gaborcsardi
Copy link
Member Author

OK, we don't really need a helper function it seems, but we can just write this inline:

#' @examples
#' \dontshow{ if (pingr::is_online()) (if (getRversion() >= "3.4") withAutoprint else force)(\{ }
#' advanced_search(Maintainer = "ORPHANED")
#' \dontshow{ \}) }

With R < 3.4 you won't get nice output from example(), but I don't think that's a big deal.

I'll make a PR for a new @exampleIf tag. We don't need to include it in the patch release, though, we can still play with it a bit.

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Nov 21, 2019

Also, pkgdown needs another look, because now I get:
https://r-hub.github.io/pkgsearch/dev/reference/pkg_search.html

It is possible that I manually tweaked the examples for the nice output, I don't remember.

So this probably won't make it into the patch release, and the PR is not that urgent...

@gaborcsardi
Copy link
Member Author

gaborcsardi commented Nov 25, 2019

I have now a branch with a PoC to add support for this in pkgdown: https://github.com/r-lib/pkgdown/compare/fix/examples-dontshow

Here is some real usage:
r-hub/pkgsearch@f64c4a0

The output looks quite good, although some empty lines might have been lost:
https://r-hub.github.io/pkgsearch/dev/reference/pkg_search.html

Eventually the roxygen \dontshow{}command will be generated from an @examplesIf tag of course.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants