moodymudskipper / flow

View and Browse Code Using Flow Diagrams
https://moodymudskipper.github.io/flow/
Other
395 stars 26 forks source link

Confusion between function environment and binding environment #115

Closed moodymudskipper closed 1 year ago

moodymudskipper commented 1 year ago

When a package has functions dynamically defined by other functions their environment might not be the namespace.

environment(waldo:::multiline)
#> <environment: namespace:waldo>
environment(waldo:::compare_by_line)
#> <environment: 0x112a303d8>
parent.env(environment(waldo:::compare_by_line))
#> <environment: namespace:waldo>

In these cases the diagram shows a confusing "{}"

flow::flow_view_deps(waldo:::compare_character, show_imports = "p", max_depth = 2)

test

And what about reexports ? They're bound in the package but their envir is another package's namespace (the latter is shown now and it makes sense).

Next to that I'd like to keep supporting functions from the global environment

So for the diagram to be intuitive we might need some heuristics that are not so intuitive.

moodymudskipper commented 1 year ago

It's more complicated than I thought. I think we handle correctly the binding environment thing.

We have :

compare_by_line <- compare_by(index_pos, extract_pos, path_line)

where compare_by is:

function (index_fun, extract_fun, path_fun) 
{
    function(x, y, paths, opts) {
        idx <- index_fun(x, y)
        if (length(idx) == 0) 
            return(character())
        x_paths <- path_fun(paths[[1]], idx)
        y_paths <- path_fun(paths[[2]], idx)
        out <- character()
        for (i in seq_along(idx)) {
            out <- c(out, compare_structure(x = extract_fun(x, 
                idx[[i]]), y = extract_fun(y, idx[[i]]), paths = c(x_paths[[i]], 
                y_paths[[i]]), opts = opts))
        }
        out
    }
}

So waldo:::compare_by_line() calls an index_fun() that is not bound in any namespace, though its environment is the namespace, since it's just waldo:::index_pos with another name.

It's a rare occurrence and probably a bad idea to do that, but how do we deal with it ? I could do some gymnastics to retrieve index_pos() but I don't know how I would show it on the diagram.

I could pretend index_fun() exists and place it on the diagram.

I could write something like "index_pos bound to index_fun" or "index_pos bound/index_fun" but that's all work and that's all weird so until somebody complains I think I will just hide these deps from the chart.

github-actions[bot] commented 9 months ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.