dgkf commented 1 year ago

While developing this, I found that there are a few features that are unfortunately quite difficult to implement. Some of which can be made significantly simpler with a few changes to the R language. This issue catalogs those changes.

Change Requests

:question: Top level pre-execution hook

Currently we have addTaskCallback to register callbacks for code to run after a top level expression, but unfortunately have no mechanism of registering code to run before each top level expression.

Without this feature, debug statements are processed only upon each top level expression's completion in the REPL, meaning that the state of the debugger in the REPL lags behind the state of the debugger in the IDE and is only synchronized after a top level callback.

With this feature, any new breakpoint traces can be set before user input is evaluated, minimizing debugger state desync.

:white_check_mark: Browser callback hooks

Currently a "shadow" browser is used, which introduces an enormous amount of complexity to the project. The browser prompt is a fixed part of the R language with almost no opportunities to inject code in its REPL. To handle this, a process is forked and its output is split between the REPL presented to the user while commands and output required for the debugger are hidden and communicate directly with the DAP server.

Instead, if hooks could be added to the browser REPL akin to the standard REPL task callbacks, then this code could be executed in the foreground process directly without cluttering the REPL output with messy server messaging calls.

Step Information in Browser Hook

Getting access to the expression that is being debugged, not just the sys.call(). Ideally with position information within the evaluating function. This seems to be accessible in-between browser steps at the C level (SrcrefPrompt, but I can't figure out how to grab this information on the R side. Update: this is available in debug() prompts, but not browser() prompts
Would be nice to have some information about what stepping commands are issued in the browser hook (n, s, c, etc)

condition argument of debug() appears to be broken?

f <- function(x) { x }
debug(f, condition = list(test = 123))
f()
#> Browse[1]> browserCondition()
#> NULL

Would love a way to disable the debug at <file>#<line>: ... text that gets spit out during each debug step now that browser hooks allow for custom-built prompts.

Other mentions of this limitation

The core challenge imposed by having a rather fixed debug browser was also felt in the vscDebugger project, which got around this challenge by running a separate R process governed by javascript. This method relies on parsing of stdout.

from https://github.com/ManuelHentschel/VSCode-R-Debugger/issues/156#issuecomment-990167766

The main problem with the vscDebugger package is that there seems to be no way to control the program flow (stepping through code while debugging) from within R itself. Therefore, I am using a node-based wrapper, which controls the input to/output from an R process underneath.

sebffischer commented 1 year ago

Have you mentioned these issues in the R-devel mailing list?

Also, I would love to somehow contribute to this project, but am not too familiar with the tools that are involved.

What would be your next steps for the debugadapter and where is it easiest to contribute? Or is your preference to first address the issues in the R language and then in this package?

dgkf commented 1 year ago

Have you mentioned these issues in the R-devel mailing list?

@sebffischer I haven't. I've never raised a feature proposal like that before, so I'm not exactly sure where to start. Is the mailing list the right place for these sorts of things?

Also, I would love to somehow contribute to this project, but am not too familiar with the tools that are involved.

Things are getting to the point where there's a reasonable expectation that it starts to work with many debug adapter servers.

You mentioned in #6 that you're using neovim - are you using mfussenegger/nvim-dap as your DAP plugin? If so, great - that's what I'm using for testing now. If not, just let me know what your plugins of choice are. Either way, I can try to write up some docs for getting started.

sebffischer commented 1 year ago

Have you mentioned these issues in the R-devel mailing list?

@sebffischer I haven't. I've never raised a feature proposal like that before, so I'm not exactly sure where to start. Is the mailing list the right place for these sorts of things?

I believe that the R-devel mailing list would be the correct address.

Also, I would love to somehow contribute to this project, but am not too familiar with the tools that are involved.

Things are getting to the point where there's a reasonable expectation that it starts to work with many debug adapter servers.

You mentioned in #6 that you're using neovim - are you using mfussenegger/nvim-dap as your DAP plugin? If so, great - that's what I'm using for testing now. If not, just let me know what your plugins of choice are. Either way, I can try to write up some docs for getting started.

Yes I am using neovim, thanks for the update in the README!

jonlachmann commented 7 months ago

Hi, really interesting project! I have been looking for a solid debug experience in R, and the best so far has been running it from A JetBrains IDE as they provide a wrapper for the whole R executable. This however sometimes becomes unstable, and I would love to start using R inside neovim.

This thread really explains a bit about why it seems so hard to get debugging in R right. I had a look at the R source code, and it is quite straightforward with how the REPL is implemented. Here we can find the main loop call to the Rf_callToplevelHandlers which provide the post evaulation callback. https://github.com/r-devel/r-svn/blob/1e9097b65026782a9b312d446eb703d4515f1e00/src/main/main.c#L271

I am thinking that adding a similar function before the evaluation should not be too hard. I however have no idea about how the R dev team feels about it, but it should be in their interest to improve the debugging experience, writing browser() in various places of the code quickly becomes annoying.

dgkf commented 7 months ago

@jonlachmann thanks for the nudge. I've drafted a few requests to the R mailing list and/or the r-contributors slack, but ultimately always felt like I must be missing something if I'm hitting this wall. I'll shoot off a message to https://r-contributors.slack.com/ in the #wishlist-discuss channel. Wish me luck & feel free to follow the conversation over there.

jonlachmann commented 7 months ago

It might be worthwhile to try implementing what is needed in the R REPL as a proof-of-concept. Running a modified version to enable proper debugging support seems the best solution until something is merged. What would be a reasonable test case to try and modify the R executable to handle?

dgkf commented 6 months ago

Thanks @sebffischer for drawing my attention to HenrikBengtsson/Wishlist-for-R.

I filed this as a proposal in https://github.com/HenrikBengtsson/Wishlist-for-R/issues/159

raffaem commented 3 months ago

@jonlachmann thanks for the nudge. I've drafted a few requests to the R mailing list and/or the r-contributors slack, but ultimately always felt like I must be missing something if I'm hitting this wall. I'll shoot off a message to https://r-contributors.slack.com/ in the #wishlist-discuss channel. Wish me luck & feel free to follow the conversation over there.

Did you manage to write to R-devel ML in the end?

I also think that would be the correct place.

Is that wishlist official?

dgkf / debugadapter

Future R Language change requests / contributions #4

Change Requests

:question: Top level pre-execution hook

:white_check_mark: Browser callback hooks

Step Information in Browser Hook

Other mentions of this limitation

from https://github.com/ManuelHentschel/VSCode-R-Debugger/issues/156#issuecomment-990167766