-
Notifications
You must be signed in to change notification settings - Fork 131
Get rid of big "Run and Debug" button for R extension #10769
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
base: main
Are you sure you want to change the base?
Conversation
… to specify how to handle Run and Debug pane
|
E2E Tests 🚀 |
| "type": "ark", | ||
| "label": "R Debugger" | ||
| "label": "R Debugger", | ||
| "languages": ["r"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change here is what keeps us from prompting folks to install an (incompatible) extension. At the very minimum, we should contribute a language here to avoid that prompt.
| "label": "R Debugger" | ||
| "label": "R Debugger", | ||
| "languages": ["r"], | ||
| "supportsUILaunch": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we work on building Positron itself, the IDE registers the vscode://schemas/vscode-extensions schema at startup. This item is not in that schema, so if we go this direction overall, we will see a diagnostic in this file moving forward:
Property supportsUILaunch is not allowed.
I did some exploration of how to silence this diagnostic but did not find anything quickly/easily.
| export const CONTEXT_DEBUGGERS_AVAILABLE = new RawContextKey<boolean>('debuggersAvailable', false, { type: 'boolean', description: nls.localize('debuggersAvailable', "True when there is at least one debug extensions active.") }); | ||
| export const CONTEXT_DEBUG_EXTENSION_AVAILABLE = new RawContextKey<boolean>('debugExtensionAvailable', true, { type: 'boolean', description: nls.localize('debugExtensionsAvailable', "True when there is at least one debug extension installed and enabled.") }); | ||
| // --- Start Positron --- | ||
| export const CONTEXT_DEBUGGER_SUPPORTS_UI_LAUNCH = new RawContextKey<boolean>('debuggerSupportsUILaunch', true, { type: 'boolean', description: nls.localize('debuggerSupportsUILaunch', "True when the debugger for the current file supports launching from the Run and Debug UI. Some debuggers (like R) use alternative debugging approaches.") }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new context key is what allows extensions to say, "Hey, don't turn on all the debugger buttons for this type of file."
lionel-
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great in R files!
Experimenting with it, I found the experience to be a bit jittery and confusing with other editors though.
https://github.com/user-attachments/assets/c857f89f-722b-4553-bf72-90b4f45fb400
Also when no editor is opened:
Should we instead look at the currently opened console, retrieve its languageId, and use that instead of the current editor's language to determine whether to show the run and debug button?
| "label": "R Debugger" | ||
| "label": "R Debugger", | ||
| "languages": ["r"], | ||
| "supportsUILaunch": false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding casing, it would be better as supportUiLaunch. The simplest and most practical rule for camel casing is that it should be possible to programmatically go from camelCase to snake_case (in this case, support_ui_launch rather than support_u_i_launch). That's also the convention used inside VS Code.
| if (language && adapterManager.someDebuggerInterestedInLanguage(language)) { | ||
| this.debugStartLanguageContext.set(language); | ||
| this.debuggerInterestedContext.set(true); | ||
| storageSevice.store(debugStartLanguageKey, language, StorageScope.WORKSPACE, StorageTarget.MACHINE); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would help to have this original code fenced
| if (language && adapterManager.someDebuggerInterestedInLanguage(language)) { | |
| this.debugStartLanguageContext.set(language); | |
| this.debuggerInterestedContext.set(true); | |
| storageSevice.store(debugStartLanguageKey, language, StorageScope.WORKSPACE, StorageTarget.MACHINE); | |
| if (language && adapterManager.someDebuggerInterestedInLanguage(language)) { | |
| // --- End Positron --- | |
| this.debugStartLanguageContext.set(language); | |
| this.debuggerInterestedContext.set(true); | |
| storageSevice.store(debugStartLanguageKey, language, StorageScope.WORKSPACE, StorageTarget.MACHINE); | |
| // --- Start Positron --- | |
| // if (language && this.debugService.getAdapterManager().someDebuggerInterestedInLanguage(language)) { | ||
| // Find debuggers for this language and check if any support UI launch | ||
| const adapterManager = this.debugService.getAdapterManager(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no changes to the if branch right? Probably better to leave this as is and refetch the adapterManager below, to reduce the amount of stuff in Positron fences.
| "view": "debug", | ||
| "contents": "Positron currently provides initial debugging support for R code.\n[Learn more](https://positron.posit.co/guide-r.html#debugging)", | ||
| "when": "debugStartLanguage == 'r'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! We'll be able to use this to provide a "Pause" button to interrupt a running R command and drop in the debugger. This would be ideal with a context key that indicates whether the current console is busy.
Addresses #3681
This is a little heavy handed (I would have liked to find a solution here that involves a less dramatic change) but this area of upstream doesn't seem to involve a lot of churn, so I think it's likely useful for us to make this type of adjustment. Quarto files are another type where this whole pane is quite confusing, and we can use a similar approach in that extension (set
supportsUILaunchand contribute aviewsWelcomebutton or something).With this PR, you now see this in the "Run and Debug" pane for
.Rfiles:You should still see the normal content for a
.pyfile or.qmdfile or another kind of file.Release Notes
New Features
Bug Fixes
QA Notes
.Rfiles..py,.qmd, or other types of files.