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

Add @examplesIf tag #993

Merged
merged 3 commits into from
Nov 9, 2020
Merged

Add @examplesIf tag #993

merged 3 commits into from
Nov 9, 2020

Conversation

gaborcsardi
Copy link
Member

Quite simple. Let me know what you think.

Closes #962.

@hadley
Copy link
Member

hadley commented Mar 5, 2020

Has anything else changed that requires us to rethink this, or should I review it as is?

@gaborcsardi
Copy link
Member Author

@hadley Good as it is, well, except that it needs NEWS and docs, if we decide to go with this. FWIW I use it in several packages and seems to work well.

@krlmlr
Copy link
Member

krlmlr commented Jun 8, 2020

If every line of code has its own if() block -- do we still need withAutoprint() and braces?

foo <- TRUE
if (foo) "foo"
#> [1] "foo"
if (foo) if (TRUE) "bar" else "baz"
#> [1] "bar"
if (foo) if (FALSE) "bar" else "baz"
#> [1] "baz"

Created on 2020-06-08 by the reprex package (v0.3.0)

@gaborcsardi
Copy link
Member Author

@krlmlr hmmm, it is no obvious to me that you can do that. E.g. how about multi-line strings? Or raw strings? You could parse the expression, but this is starting to get much more complicated than withAutoprint(). I am not even sure what is wrong with withAutoprint()?

@krlmlr
Copy link
Member

krlmlr commented Jun 8, 2020

Got you. I thought this pattern is applied to every statement. In this case withAutoprint() is just fine. I'll test-drive in my packages.

@gaborcsardi
Copy link
Member Author

@krlmlr Thanks!

krlmlr added a commit to cynkra/dm that referenced this pull request Jul 2, 2020
krlmlr added a commit to cynkra/dm that referenced this pull request Jul 2, 2020
@hadley
Copy link
Member

hadley commented Jul 20, 2020

Can you add a news bullet? I'd like to merge this just so we can experiment a bit more with it (without advertising it heavily)

@gaborcsardi gaborcsardi reopened this Jul 20, 2020
@gaborcsardi
Copy link
Member Author

gaborcsardi commented Jul 20, 2020

(Closed by mistake.)

@hadley So, this version does not handle examples in R6 classes, which are special. It would need this commit to handle them: gaborcsardi/roxygenlabs@fc249f5

Should I add that?

@hadley
Copy link
Member

hadley commented Jul 20, 2020

I think we should leave R6 for now

@gaborcsardi
Copy link
Member Author

@hadley Added NEWS bullet.

@gaborcsardi gaborcsardi merged commit 513c9e2 into master Nov 9, 2020
@gaborcsardi gaborcsardi deleted the feature/examplesIf branch November 9, 2020 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic examples, take two
3 participants