flow_run breaks with functions like match.arg(), formals(), sys.call(), match.call(), on.exit(), Recall()...

[moodymudskipper]

I thought I didn't need it because I could populate an environment with promises and even calls like browser() would work. But calls to match.arg() and formals() (with no argument) will not behave well with the current system unless we hack their definitions...

Good news is that we know it's possible, bad news is we have to dive into this mess again.

I realize some things won't work though, or will need extra care and won't be trivial at all : on.exit() as is will run at the end of the chunk,not what we want at all. Also, sys.call(), match.call() will be wrong, I don't know how I've missed that.

• match.arg() is the number one issue because it's very common, but will be solved cleanly
• formals() will be solved by the same solution
• sys.call() and match.call() will be almost solved, in their basic form they will work if the temp functions are renamed as the original, but it's not robust
• on.exit could be caught and executed after we exit the function

It's all clunky heuristics and I don't like it too much, if you're debugging you don't want the debugger to bring its own bugs. One solution would be to refuse every problematic function, but they could be called through another call with eval.parent(quote(...)) so even this is not robust...

We can also kill the functionality but that'd be a shame because it's useful and works well in most cases, so probably better to have an explicit disclaimer, and specific warnings when problematic calls are encountered.

Let's see what covr does before anythin, maybe there's a more proper way. It should be explained here: https://www.youtube.com/watch?v=wP82XSFEiYs

[moodymudskipper]

Here's what covr does :

I think we can do something like this, delimitate the blocks (not the calls like in covr), and insert a call to flow:::update(n, redraw = TRUE) where n is the number of the block and the call would update the edge and node data and optionally redraw.

when using browse = TRUE we would just debug this function, the calls to flow:::update(n) (no need for explicit redraw) wouldn't pollute too much the vision.

when using browse = n we could insert a browser call at the right place, or even better, have in flow:::update() a call like :

if (n ==  browse) on.exit( browser() )

The browser value would be brought in by some environment magic to be defined, or we'd have the update function built on the spot with hardcoded value, or we use a global variable in flow's namespace (we'r only browsing one function at a time even if we nest flow_run calls)

I think for clarity we should only add these flow:::update() calls in code blocks and not in control flow, because this looks ugly, and might confuse some users :

if({flow:::update(12)
  cond}) {

}

If we want to get really fancy control flow can be redefined to call the updates too.

This solves all the problems outlined in post above, and make it all less of a hack and we can us all the browser buttons in Rstudio, an additional benefit is that we go through loops, so we can outline in for loops only the paths that have been taken, and we can even count how many times each path was taken.

[moodymudskipper]

All these should be solved by flow_run2()

[moodymudskipper]

now all implemented flow_run()

[github-actions[bot]]

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.