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

Include comments when constructing functions? #58

Closed
krlmlr opened this issue Oct 13, 2022 · 5 comments
Closed

Include comments when constructing functions? #58

krlmlr opened this issue Oct 13, 2022 · 5 comments
Milestone

Comments

@krlmlr
Copy link
Contributor

krlmlr commented Oct 13, 2022

r-dbi/RSQLite#437 is generated by r-dbi/DBItest#267 which uses constructive. In DBItest I'm making extensive use of inline comments. Is there a way to keep them with construct() ?

@moodymudskipper
Copy link
Collaborator

moodymudskipper commented Oct 13, 2022 via email

@krlmlr
Copy link
Contributor Author

krlmlr commented Oct 14, 2022

Right, I'd switch to capturing the body then.

We can still parse + style, though?

@moodymudskipper
Copy link
Collaborator

foo <- function(x) {
  # foo
  x
}

library(constructive)

# we have
construct(foo)
#> {constructive} couldn't create code that reproduces perfectly the input
#> ℹ Call `construct_issues()` to inspect the last issues
#> function(x) {
#>   x
#> }

# we can construct with the srcref
construct(foo, opts_function(srcref = TRUE))
#> {constructive} couldn't create code that reproduces perfectly the input
#> ℹ Call `construct_issues()` to inspect the last issues
#> (function(x) {
#>   x
#> }) |>
#>   structure(
#>     srcref = c(1L, 8L, 4L, 1L, 8L, 1L, 1L, 4L) |>
#>       structure(
#>         srcfile = list2env(
#>           list(
#>             fixedNewlines = TRUE,
#>             lines = c("foo <- function(x) {", "  # foo", "  x", "}", ""),
#>             filename = ""
#>           ),
#>           parent = .GlobalEnv
#>         ) |>
#>           structure(class = c("srcfilecopy", "srcfile")),
#>         class = "srcref"
#>       )
#>   )

# and we can verify that it prints properly
(function(x) {
  x
}) |>
  structure(
    srcref = c(1L, 8L, 4L, 1L, 8L, 1L, 1L, 4L) |>
      structure(
        srcfile = list2env(
          list(
            fixedNewlines = TRUE,
            lines = c("foo <- function(x) {", "  # foo", "  x", "}", ""),
            filename = ""
          ),
          parent = .GlobalEnv
        ) |>
          structure(class = c("srcfilecopy", "srcfile")),
        class = "srcref"
      )
  )
#> function(x) {
#>   # foo
#>   x
#> }

# but the compact and proper way would be to reproduce the idiomatic definition with the comments and get:
function(x) {
  # foo
  x
}

# then I think we just need, though to be safe we should parse it and check that get the same language tree
as.character(attr(foo, "srcref"))
#> [1] "function(x) {" "  # foo"       "  x"           "}"

# This is only for the `opts_function("function")` (the default), for `as.function()` for instance we lose the 
# comments so we cannot use this shortcut to set the secret
as.function(alist(x=, {
  # foo
  x
}))
#> function (x) 
#> {
#>     x
#> }

# For language objects We need `opts_language(srcref=)` because now we cannot faithfully reconstruct the body in isolation
construct(body(foo))
#> quote({
#>   x
#> })
attributes(body(foo))
#> $srcref
#> $srcref[[1]]
#> {
#> 
#> $srcref[[2]]
#> x
#> 
#> 
#> $srcfile
#> <text> 
#> 
#> $wholeSrcref
#> foo <- function(x) {
#>   # foo
#>   x
#> }

# however for that case we cannot reproduce the srcref without setting attributes, because the srcref
# either doesn't reproduce the object, as above, or contains too much, as below
bar <- identity(quote({
  # foo
  x
}))

attributes(bar)
#> $srcref
#> $srcref[[1]]
#> {
#> 
#> $srcref[[2]]
#> x
#> 
#> 
#> $srcfile
#> <text> 
#> 
#> $wholeSrcref
#> bar <- identity(quote({
#>   # foo
#>   x
#> }
as.character(attr(bar, "wholeSrcref"))
#> [1] "bar <- identity(quote({" "  # foo"                
#> [3] "  x"                     "}"

@moodymudskipper
Copy link
Collaborator

moodymudskipper commented Oct 14, 2022

@krlmlr does this now works for you ? it's pushed to f-48-... as part of #56

If the srcref matches the function's definition (as measured with identical, ignoring env and srcref) it is used, if not we deparse the body.

x <- function(x) {
  # foo
  x
}

constructive::construct(x)
#> function(x) {
#>   # foo
#>   x
#> }
constructive::construct(list(a= x))
#> list(
#>   a = function(x) {
#>     # foo
#>     x
#>   }
#> )

Created on 2022-10-14 with reprex v2.0.2

Re:

We can still parse + style, though?

I don't fully understand, but styling (indention and pretty code) is done through styler in any case, and since it uses getParseData it doesn't destroy comments.

@krlmlr
Copy link
Contributor Author

krlmlr commented Oct 15, 2022

Thanks, works beautifully!

@krlmlr krlmlr closed this as completed Oct 15, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants