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

R6 instantiation #59

Merged
merged 2 commits into from
Sep 24, 2020
Merged

R6 instantiation #59

merged 2 commits into from
Sep 24, 2020

Conversation

maelle
Copy link
Contributor

@maelle maelle commented Sep 21, 2020

I wanted to see how I could get

downlit::highlight("library(crul); HttpClient$new()")
#> [1] "<span class='kr'><a href='https://rdrr.io/r/base/library.html'>library</a></span><span class='o'>(</span><span class='nv'><a href='https://docs.ropensci.org/crul'>crul</a></span><span class='o'>)</span>; <span class='nv'><a href='https://docs.ropensci.org/crul/reference/HttpClient.html'>HttpClient</a></span><span class='o'>$</span><span class='nf'>new</span><span class='o'>(</span><span class='o'>)</span>"

Created on 2020-09-21 by the reprex package (v0.3.0.9001)

But

  • it might be adding too much complexity just for the special case of R6 instantiation;
  • I can't test it without adding a dependency (or some sort of mocking);
  • The usefulness of this is quite limited since downlit doesn't know what packages are really attached (compared to what knitr would know) so I'd probably be better off just namespacing (the reprex below works without any tweak). 😅
downlit::highlight("crul::HttpClient$new()")
#> [1] "<span class='nf'>crul</span><span class='nf'>::</span><span class='nv'><a href='https://docs.ropensci.org/crul/reference/HttpClient.html'>HttpClient</a></span><span class='o'>$</span><span class='nf'>new</span><span class='o'>(</span><span class='o'>)</span>"

Created on 2020-09-21 by the reprex package (v0.3.0.9001)

@hadley
Copy link
Member

hadley commented Sep 21, 2020

downlit should know what packages are attached (subject to some minor heuristics); it keeps a record of all uses of library() and require().

@maelle
Copy link
Contributor Author

maelle commented Sep 21, 2020

right but it does that only for the chunk/code at hand, right? whereas knitr would know for all chunks until the one at hand? or am I missing something?

@maelle
Copy link
Contributor Author

maelle commented Sep 21, 2020

oooh no I see, sorry, I did miss something.

@maelle
Copy link
Contributor Author

maelle commented Sep 21, 2020

So that leaves two arguments against this feature:

  • it might be adding too much complexity just for the special case of R6 instantiation;
  • I can't test it without adding a dependency (or some sort of mocking).

R/highlight.R Outdated Show resolved Hide resolved
@maelle maelle marked this pull request as ready for review September 22, 2020 11:04
@hadley hadley merged commit b6572c9 into r-lib:master Sep 24, 2020
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.

2 participants