Robustness of stdout parsing #13

sebffischer commented 2023年02月06日 13:19:03 +01:00 (Migrated from github.com)

I think the mechanism of reading stdout from the process that runs browser browser() - which is currently done using read_until_browse_prompt function - should be replaced with something else. It is possible that something calls print("Browse[1] ") and then things would break badly. Probably this does not have too high priority but I wanted to mention it.

A solution would be that the process that runs browser() does not redirect its output directly, but that it gets the commands that it should execute wrapped in a function.
For example, the browser() process does not get the expression print(x) into its stdin, but send_back_to_child(print(x)). And the send_backt_to_child (should have a weirder name to avoid any name clashes) captures the output and then sends it back to the child via a pipe.

What do you think?

To do this, one would have to check before sending the command whether the command that is being sent is a standard R expression to be evaluated or a browser control symbol such as "n" or "cont" etc.

I think the mechanism of reading stdout from the process that runs browser `browser()` - which is currently done using `read_until_browse_prompt` function - should be replaced with something else. It is possible that something calls `print("Browse[1] ")` and then things would break badly. Probably this does not have too high priority but I wanted to mention it. A solution would be that the process that runs `browser()` does not redirect its output directly, but that it gets the commands that it should execute wrapped in a function. For example, the `browser()` process does not get the expression `print(x)` into its stdin, but `send_back_to_child(print(x))`. And the `send_backt_to_child` (should have a weirder name to avoid any name clashes) captures the output and then sends it back to the child via a pipe. What do you think? To do this, one would have to check before sending the command whether the command that is being sent is a standard R expression to be evaluated or a browser control symbol such as `"n"` or `"cont"` etc.

sebffischer commented 2023年02月12日 16:06:23 +01:00 (Migrated from github.com)

Maybe a simpler solution is to add a TaskCallback to the process that runs the browser that does something like cat(".______before_next_browser_propt\n"), that can be used in the read_until_browse_prompt function.

Maybe a simpler solution is to add a `TaskCallback` to the process that runs the browser that does something like `cat(".______before_next_browser_propt\n")`, that can be used in the `read_until_browse_prompt` function.

dgkf commented 2023年02月13日 20:08:48 +01:00 (Migrated from github.com)

Maybe a simpler solution is to add a TaskCallback

This is a great idea! It was weird to discover that task callbacks get called while in browser(), but maybe we can exploit it to handle some debugging steps.

Just to document the behavior a bit:

• callbacks registered with addTaskCallback() get called after R code is executed in the browser()
• this does not include things like debugger steps (n, cont, etc)

But this might be enough for us. For anything that isn't a debugger command (n, cont, etc), we already have the hook we need to execute arbitrary code as a task callback. For anything that is a debugger command, we can monitor the input to hide a debugger update. Since this is a much narrower set of inputs, this would be much easier to handle holistically.

---

example:

x <- function() { browser(); 1 }
addTaskCallback(function(...) { cat("<synchronize debugger here [", sys.nframe(), "]>", "\n", sep = ""); TRUE })
x()
# Called from: x() # 1. when using `shadow_browser`, inject sync code
# Browse[1]> "test"
# [1] "test"
# <synchronize debugger here [2]> # 2.2 automatically sync on user code input
# Browse[1]> # 2.1 no entry, same as `n`, need to inject sync code
# debug at #1: [1] 1
# Browse[2]> 
# [1] 1
# <synchronize debugger here [1]> # 3. can self-terminate callback at top level (nframe == 1)
# >

1. Just as it is now, kick off a new task callback when a browser starts
2. We'd still need to capture user input, but instead of blindly reading until the next browser prompt we only check for whether a debug keyword was entered
   1. If a keyword is entered, perform the keyword (n, cont, etc) and sync manually, hiding execution from the user
   2. Otherwise, the task callback handles the syncing - no extra work needed
3. Self terminate the task callback when we reach the top level

> Maybe a simpler solution is to add a `TaskCallback` This is a great idea! It was weird to discover that task callbacks get called while in `browser()`, but maybe we can exploit it to handle some debugging steps. Just to document the behavior a bit: - callbacks registered with `addTaskCallback()` get called after R code is executed in the `browser()` - this does not include things like debugger steps (`n`, `cont`, etc) But this might be enough for us. For anything that isn't a debugger command (`n`, `cont`, etc), we already have the hook we need to execute arbitrary code as a task callback. For anything that _is_ a debugger command, we can monitor the input to hide a debugger update. Since this is a much narrower set of inputs, this would be much easier to handle holistically. --- example: ```r x <- function() { browser(); 1 } addTaskCallback(function(...) { cat("<synchronize debugger here [", sys.nframe(), "]>", "\n", sep = ""); TRUE }) x() # Called from: x() # 1. when using `shadow_browser`, inject sync code # Browse[1]> "test" # [1] "test" # <synchronize debugger here [2]> # 2.2 automatically sync on user code input # Browse[1]> # 2.1 no entry, same as `n`, need to inject sync code # debug at #1: [1] 1 # Browse[2]> # [1] 1 # <synchronize debugger here [1]> # 3. can self-terminate callback at top level (nframe == 1) # > ``` 1. Just as it is now, kick off a new task callback when a browser starts 1. We'd still need to capture user input, but instead of blindly reading until the next browser prompt we only check for whether a debug keyword was entered 1. If a keyword is entered, perform the keyword (`n`, `cont`, etc) and sync manually, hiding execution from the user 1. Otherwise, the task callback handles the syncing - no extra work needed 1. Self terminate the task callback when we reach the top level

dgkf commented 2024年07月02日 05:27:40 +02:00 (Migrated from github.com)

Now that we have the browser hook, we can much more easily capture browser steps (n, c), but we still need the task callback to emit environment update messages, so this will still be a useful suggestion.

Now that we have the browser hook, we can much more easily capture browser steps (`n`, `c`), but we still need the task callback to emit environment update messages, so this will still be a useful suggestion.