FR: allow flow_view to parse "withr" or "suppress" family of functions

[zkamvar]

I notice that if I have a {withr}-like pattern in my code or use a suppress family function, {flow} cannot disentangle it.

I will occasionally have functions that look like this:

fun <- function(path, ...) {
  # ... 
  withr::with_dir(path, {
    # ... 
  })
  # ...
}

When flow encounters the {withr} call, it does not parse the expression. The same is true for calls to suppressMessages(). As a reproducible example, here is something should produce a simple if/else flow chart:

f <- function() {
  suppressMessages({
    if (getOption("a.test") == 1L) print("1") else print("2")
})
}

flow::flow_view(f)

^{Created on 2023-07-10 with reprex v2.0.2}

That being said, I really like {flow}! I'm in the process of onboarding new developers and being able to create these visuals to walk through the logic is super helpful!

[moodymudskipper]

I like the idea but I wonder what this would look like and how it would work.

Functions are common offenders for this situation and I implemented the nested_fun argument for those.

We might do something like this :

But then what do we do with : expression() or tryCatch() that might nest several of those in a single call?

expression(if(...) ..., if(...) ...)
tryCatch(if(...) ..., error = function(e) if(...) ...)

Maybe this would be a more general way, so if we have a several nested inputs they would be called 1*, 1** etc:

Or the other way around

I think I like the latter better because it reads from top to bottom more naturally.

The rule would probably be that whenever control flow is found in an argument the full argument is expanded like this. function definitions might not need special casing.

In few cases when we have deeper nesting of those this might still work, we'd have another box in the old box, and if we use index as in examples above the box indices don't change compared to current situation so this might even be tackled as an ulterior step, so we don't have to dig into the algorithm too much.

Then we'd have to choose if we make it optional, but probably have it work always like this would be fine.

@zkamvar How do these proposals above look ?

[zkamvar]

Thank you for the quick reply! I agree with your assessment and am very much a fan of the latter proposal---the top-down arrangement makes it much easier to read and the box with the starred index gives is just the right amount of context for me to know what's going on.

[moodymudskipper]

Thanks. This has good value I think so should be a priority in this package, but I don't have a lot of time for it these days so it'll have to wait a bit.

Tests cases should be the given example first, then bquote() because it has 2 functions to expand in the same block, and one function that we don't need to expand :

flow::flow_view(bquote)
#> We found function definitions in this code, use the argument nested_fun to inspect them
#> $unquote
#> function(e) {
#>     if (is.pairlist(e)) 
#>         as.pairlist(lapply(e, unquote))
#>     else if (is.call(e)) {
#>         if (is.name(e[[1L]]) && as.character(e[[1]]) == ".") 
#>             eval(e[[2L]], where)
#>         else if (splice) {
#>             if (is.name(e[[1L]]) && as.character(e[[1L]]) == 
#>                 "..") 
#>                 stop("can only splice inside a call", call. = FALSE)
#>             else as.call(unquote.list(e))
#>         }
#>         else as.call(lapply(e, unquote))
#>     }
#>     else e
#> }
#> 
#> $is.splice.macro
#> function(e) is.call(e) && is.name(e[[1L]]) && as.character(e[[1L]]) == 
#>     ".."
#> 
#> $unquote.list
#> function(e) {
#>     p <- Position(is.splice.macro, e, nomatch = NULL)
#>     if (is.null(p)) 
#>         lapply(e, unquote)
#>     else {
#>         n <- length(e)
#>         head <- if (p == 1) 
#>             NULL
#>         else e[1:(p - 1)]
#>         tail <- if (p == n) 
#>             NULL
#>         else e[(p + 1):n]
#>         macro <- e[[p]]
#>         mexp <- eval(macro[[2L]], where)
#>         if (!is.vector(mexp) && !is.expression(mexp)) 
#>             stop("can only splice vectors")
#>         c(lapply(head, unquote), mexp, as.list(unquote.list(tail)))
#>     }
#> }

^{Created on 2023-07-12 with reprex v2.0.2}