Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the Ark DAP #1135

Merged
merged 12 commits into from
Sep 4, 2023
Merged

Add support for the Ark DAP #1135

merged 12 commits into from
Sep 4, 2023

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented Aug 23, 2023

This PR adds support for the Ark DAP server implemented in posit-dev/ark#79.

Design

The connection is managed via a Jupyter comm just like the LSP. However I tried to simplify the design a bit:

  • The resources are fully managed by the backend and by the DAP client. Positron does not maintain any state or resource besides the DAP comm which is always running.

  • When Ark reloads, the comm adapter gets cleaned up the usual way. When Positron refreshes, we currently start a new DAP comm, but I'm hoping this will be resolved in Jupyter: Refreshing the UI starts new comms instead of reusing existing ones #1126.

We might need something more complex to support other kinds of DAP but I thought we could start with this simple setup.

Workflow

I tried to follow these goals:

  • The user doesn't need to attach manually. The debugger gets attached automatically when Ark hits a browser() prompt. Then the debug toolbar becomes visible, otherwise it is hidden when the R session is not paused in the debugger.

    This is currently achieved by sending a comm event when a browser() prompt is detected. This event causes Positron to start a debug session with vscode.debug.startDebugging().

    I haven't investigated how to support breakpoints yet. It's possible that we'll have to keep the debug session on at all times in order to add hooks that let us insert the relevant browser() calls. I noticed there is an option we might be able to use to hide the debug toolbar while R is not paused in the debugger, but I haven't explored that yet. Alternatively, there might exist a more direct VS Code API for hooking into the breakpoint UI. This would be ideal.

  • The toolbar controls are mapped to actions that make sense in the context of a debugger that is not running separately from the debugged program. This concerns the quit and restart commands.

    The quit button should normally not affect the debugged program. However since the debugger and the REPL are one and the same, and I think the user experience is better if Positron is always in debug mode while in a browser prompt, I mapped it to Q. This causes the current R stack to terminate and go back to top-level, and in turn this causes the debug session to quit.

    The restart button should normally restart the debugger. Since that does not really make sense in our context, I made it a shortcut for restarting the runtime. If we don't override it this way, we get the default behaviour of disconnecting, which we interpret as a Q, and reconnecting, which makes the debug toolbar show up even though we're no longer in a browser prompt. I thought this was confusing and so came up with the alternative behaviour instead. Alternatively we could send an upstream PR to make it possible to hide the button, or just patch VS code to do it ourselves.

Events

There are a number of actions that need to be performed from Positron and which are triggered by comm events. I've added a new onDidReceiveCommMsg() emitter to the Runtime Client Adapter in order to support handling these events from the Jupyter Adapter.

  • start_debug: This event is emitted when a browser() prompt is detected. It causes Positron to start the DAP client and connect via TCP to the DAP backend.

  • execute: When the user interacts with R through the debug toolbar (e.g. next, step-in, etc), we need to perform the corresponding action from Positron rather than from Ark. So this event instructs Positron to visibly execute the command. This provides feedback about what is happening and allows the prompt to update as needed in the console UI.

    If Ark is not connected to Positron, the commands are sent to the ReadConsole() method directly.

  • restart: Used to restart the runtime when the user clicks the restart button in the debug toolbar.

TODO

Main features that are missing:

  • Variables pane

  • Breakpoints (see discussion above)

  • Automatic srcref instrumentation in dummy files when source references are missing

With globalCallingHandlers(), it is possible to interrupt R into browser() at any point. This could be useful for debugging infloops. However it's unclear how the user should activate such a feature, given that we avoid showing the debug toolbar (and thus the pause button) when R is not paused in the debugger.

extensions/jupyter-adapter/src/LanguageRuntimeAdapter.ts Outdated Show resolved Hide resolved
// (QueryInterface style) instead of just demanding it?
//
// The Jupyter kernel spec does not provide a way to query for
// supported comms; the only way to know is to try to create one.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have this problem in a few other places and I haven't decided how best to solve it. I wonder if it makes the most sense to have it returned along with (for instance) the startup banner in LanguageRuntimeInfo? (Not something we need to fix in this PR)

@jmcphers
Copy link
Collaborator

Sorry for the delay; this looks good!

@lionel- lionel- merged commit 1a9337e into main Sep 4, 2023
1 check passed
@lionel- lionel- deleted the feature/r-dap branch September 4, 2023 10:27
@lionel- lionel- mentioned this pull request Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants