1
1
Fork
You've already forked debugadapter
0

Effective immediately! Drop support for stable R 😝 #15

Open
opened 2024年06月28日 17:09:59 +02:00 by dgkf · 5 comments
dgkf commented 2024年06月28日 17:09:59 +02:00 (Migrated from github.com)
Copy link

This project hit a few constraints that made it mostly dead-on-arrival. Most notably, the lack of any hooks for interacting with an ongoing browser session.

Well that looks like it's unofficially been solved in bugs.r-project.org discussion.

We now have an experimental global option to add a browser "error handler", which allows us to hook into conditions intercepted by the browser.

I see this as almost an inevitability in the R world, so I'm willing to stake the project against the eventual inclusion of such a feature. Instead of even entertaining support for current versions of R and contorting the project to work in that scheme, I think it's better to throw all that out and use this much more direct mechanism of intercepting the debugger.

This project hit a few constraints that made it mostly dead-on-arrival. Most notably, the lack of any hooks for interacting with an ongoing browser session. Well that looks like it's unofficially been solved in [bugs.r-project.org discussion](https://bugs.r-project.org/show_bug.cgi?id=18590). We now have an experimental global option to add a browser "error handler", which allows us to hook into conditions intercepted by the browser. I see this as almost an inevitability in the R world, so I'm willing to stake the project against the eventual inclusion of such a feature. Instead of even entertaining support for current versions of R and contorting the project to work in that scheme, I think it's better to throw all that out and use this much more direct mechanism of intercepting the debugger.
dgkf commented 2024年06月28日 19:39:36 +02:00 (Migrated from github.com)
Copy link
- [original r-project bugs discussion](https://bugs.r-project.org/show_bug.cgi?id=18590) - [@ltierney example on how to use browser hooks](https://github.com/ltierney/browserhook) - [r-source commits related to browser hooks](https://github.com/search?q=repo%3Awch%2Fr-source+BROWSER_HOOK&type=commits)
dgkf commented 2024年06月28日 21:24:15 +02:00 (Migrated from github.com)
Copy link

Note, this probably means the next stage of this project will only work on R-devel, compiled with the -DUSE_BROWSER_HOOK flag set. Because I build R from source ever few years -- leaving just enough time to completely forget what I'm doing -- I'm going to document my exact steps:

svn checkout https://svn.r-project.org/R/trunk R
cd R
# fetch recommended packages
./tools/rsync-recommended
# make a build target directory
mkdir target
cd target
# in my case, i'm on wayland with no x11
../configure --without-x CPPFLAGS=-DUSE_BROWSER_HOOK
make
# finally, launch our browser-hook-enabled R
bin/R
Note, this probably means the next stage of this project will only work on `R-devel`, compiled with the `-DUSE_BROWSER_HOOK` flag set. Because I build R from source ever few years -- leaving just enough time to completely forget what I'm doing -- I'm going to document my exact steps: ```sh svn checkout https://svn.r-project.org/R/trunk R cd R # fetch recommended packages ./tools/rsync-recommended # make a build target directory mkdir target cd target # in my case, i'm on wayland with no x11 ../configure --without-x CPPFLAGS=-DUSE_BROWSER_HOOK make # finally, launch our browser-hook-enabled R bin/R ```
dgkf commented 2024年06月29日 21:20:30 +02:00 (Migrated from github.com)
Copy link

Taking some notes on Luke's browser hooks implementation. More of a "user acceptance testing" style feedback, with my own thoughts on this as an API for hooking into the browser:

Impressions

Overall, the design feels like it's at odds with itself. The hook is used to catch entering into the browser, automatically resets the hook, then to actually enter a browser while intercepting future browser steps you have to reintroduce the hook and call browser with a ignoreHook flag. My impulsive response to such a design is that it feels overly complicated for what it allows.

Further Questions / Suggestions

  • Like top-level callbacks, would prefer if this hook used a logical return value to decide whether to remove this hook for future calls or left it to the user to erase it from options() instead of forcibly removing it.

  • The hook "burns after reading". It automatically gets changed to NULL after being invoked, which is quite unintuitive. It also means the hook must accept hook as its first argument as a way of propagating itself.

  • browser getting an ignoreHook argument feels a bit heavy handed (and it is not included in the formals() in the R API). Would prefer to always use the hook, but allow developers to juggle what hook is set using options() and on.exit() (to restore a hook). This style of scope management feels more much idiomatic within R.

  • Instead of using finally to catch exits, would be preferred if browser() exited with a proper condition. Exiting with a ^C interrupt exits with a c("interrupt", "condition"), whereas exiting with a Browse[1]> Q seems to long-jump to the top level without a condition... at least in my tests using partial matching to try to catch anything that might be unofficially returned. it seems finally is the only way to catch a Q exit.

  • It would be nice if options(prompt) and options(continue) could be used within the Browse[1]> to make a custom debug REPL. Currently hard-coded, but with this hook becomes a useful design surface. Defaults could of course be set such that the current Browse[n]> prompt is shown, but it would be great if this was customizable.

  • Within a browser() session, is there a way to get the current debug at position? Ideally similar to utils::findLineNum? Trying to track expression location through a browser path would be redundant and error prone. Seems like this might be exposed through the C API?

  • Luke's example returns the output of browser()... does this ever return anything useful?

  • Interestingly, with this new code, calling browser(ignoreHook = TRUE) provides an interface for long-jumping to the topenv within R, forcing the early termination all the frames on the stack. Almost certainly not an intended feature, but an interesting one!


tagging @lionel- - maybe interesting to you if you're hoping to stabilize this feature for your DAP implementation

Taking some notes on Luke's browser hooks implementation. More of a "user acceptance testing" style feedback, with my own thoughts on this as an API for hooking into the browser: ### Impressions Overall, the design feels like it's at odds with itself. The hook is used to catch _entering_ into the browser, automatically resets the hook, then to actually enter a browser while intercepting future browser steps you have to reintroduce the hook and call browser with a `ignoreHook` flag. My impulsive response to such a design is that it feels overly complicated for what it allows. ### Further Questions / Suggestions - Like top-level callbacks, would prefer if this hook used a logical return value to decide whether to remove this hook for future calls or left it to the user to erase it from `options()` instead of forcibly removing it. - The hook "burns after reading". It [automatically gets changed to `NULL`](https://github.com/wch/r-source/commit/6691d53058214c16e5ab8fd6eb41c06e7fb3227f#diff-5ea4348bab8a5f5ce95785d05287a72869f48a3d364429b1580326b09f39da35R1346) after being invoked, which is quite unintuitive. It also means the hook must accept `hook` as its first argument as a way of propagating itself. - `browser` getting an `ignoreHook` argument feels a bit heavy handed (and it is not included in the `formals()` in the R API). Would prefer to always use the hook, but allow developers to juggle what hook is set using `options()` and `on.exit()` (to restore a hook). This style of scope management feels more much idiomatic within R. - Instead of using `finally` to catch exits, would be preferred if `browser()` exited with a proper condition. Exiting with a `^C` interrupt exits with a `c("interrupt", "condition")`, whereas exiting with a `Browse[1]> Q` seems to long-jump to the top level without a condition... at least in my tests using partial matching to try to catch anything that might be unofficially returned. it seems `finally` is the only way to catch a `Q` exit. - It would be nice if `options(prompt)` and `options(continue)` could be used within the `Browse[1]>` to make a custom debug REPL. Currently hard-coded, but with this hook becomes a useful design surface. Defaults could of course be set such that the current `Browse[n]>` prompt is shown, but it would be great if this was customizable. - Within a `browser()` session, is there a way to get the current `debug at` position? Ideally similar to `utils::findLineNum`? Trying to track expression location through a browser path would be redundant and error prone. Seems like this might be [exposed through the C API](https://github.com/wch/r-source/blob/cdc1afa836d18b31ef9104da0b4fa89b08ad83c5/src/main/debug.c#L119)? - Luke's example returns the output of `browser()`... does this ever return anything useful? - Interestingly, with this new code, calling `browser(ignoreHook = TRUE)` provides an interface for long-jumping to the topenv within R, forcing the early termination all the frames on the stack. Almost certainly not an intended feature, but an interesting one! --- tagging @lionel- - maybe interesting to you if you're hoping to stabilize this feature for your DAP implementation
PMassicotte commented 2024年07月01日 09:59:33 +02:00 (Migrated from github.com)
Copy link

Cool to see it reviving 👍 Just for my understanding, is it supposed to currently work in its current state? I have tried to use it a couple of times without success.

Cool to see it reviving :+1: Just for my understanding, is it supposed to currently work in its current state? I have tried to use it a couple of times without success.
dgkf commented 2024年07月01日 16:15:28 +02:00 (Migrated from github.com)
Copy link

is it supposed to currently work [within neovim] in its current state?

When I was working in this, neovim was what I used to test to see if it was working. There were quite a few very specific steps to get it working - I needed to use a fork of processx where I enabled intercepting of the stdin file handle and it would only work on unix-like platforms where processes can be forked.

This iteration will require a devel build of R with the -DUSE_BROWSER_HOOK flag, which is its own burden on the project, but it will be cross platform and less flaky.

As soon as I have a minimal example I’ll write up new config steps for neovim and hopefully helix.

> is it supposed to currently work [within neovim] in its current state? When I was working in this, neovim was what I used to test to see if it was working. There were quite a few very specific steps to get it working - I needed to use a fork of processx where I enabled intercepting of the stdin file handle and it would only work on unix-like platforms where processes can be forked. This iteration will require a devel build of R with the `-DUSE_BROWSER_HOOK` flag, which is its own burden on the project, but it will be cross platform and less flaky. As soon as I have a minimal example I’ll write up new config steps for neovim and hopefully helix.
Sign in to join this conversation.
No Branch/Tag specified
main
add-woodpecker-ci
ci-sync-test
dev/launch-mode-2
dev/s7-test
dev/browser-relay
No results found.
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dgkf/debugadapter#15
Reference in a new issue
dgkf/debugadapter
No description provided.
Delete branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?