Skip to content

Change C_Cpp.debugShortcut to resource scope#9514

Merged
sean-mcmanus merged 9 commits intomainfrom
seanmcm/debugShortcutLanguageOverridable
Jun 29, 2022
Merged

Change C_Cpp.debugShortcut to resource scope#9514
sean-mcmanus merged 9 commits intomainfrom
seanmcm/debugShortcutLanguageOverridable

Conversation

@sean-mcmanus
Copy link
Copy Markdown
Contributor

Enables a potential fix for platformio/platformio-vscode-ide#3229 .

@bobbrow
Copy link
Copy Markdown
Member

bobbrow commented Jun 28, 2022

This will disable it for all [c] files if that Platform.io extension is installed though. Is that what we want?

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

@bobbrow That would disable it by default -- users could add and modify the setting themselves to re-enable it. Another option would be to make it a per-workspace or per-workspace folder setting.

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

@bobbrow That would disable it by default -- users could add and modify the setting themselves to re-enable it. Another option would be to make it a per-workspace or per-workspace folder setting.

Changing it to "language-override" is the "least permissive" solution, so we can change it to "window" or "resource" scope later on without breaking people if we find there is sufficient reason to.

@bobbrow
Copy link
Copy Markdown
Member

bobbrow commented Jun 28, 2022

What I mean is that if the platform.io extension so much as activates (even in a workspace that does not have a platform.io project), no [c] files in that workspace will have the run/debug button visible by default.

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

What I mean is that if the platform.io extension so much as activates (even in a workspace that does not have a platform.io project), no [c] files in that workspace will have the run/debug button visible by default.

Yeah, that seems fine to me if that's what the PlatformIO extension wants -- users should be able to override it with

"[c]": {
"C_Cpp.debugShortcut": true
}

if they really want.

@bobbrow
Copy link
Copy Markdown
Member

bobbrow commented Jun 29, 2022

I know users can override it. I'm just saying that we risk getting more issues opened in our repository as a result. I don't know how big their user base is because they generate c_cpp_properties.json instead of using the config provider, but maybe we could hide the button based on some other information they provide. I'm more just hoping that overriding the setting in their package.json would be a temporary fix as opposed to a long term one.

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

sean-mcmanus commented Jun 29, 2022

I know users can override it. I'm just saying that we risk getting more issues opened in our repository as a result. I don't know how big their user base is because they generate c_cpp_properties.json instead of using the config provider, but maybe we could hide the button based on some other information they provide. I'm more just hoping that overriding the setting in their package.json would be a temporary fix as opposed to a long term one.

Yeah, it could be a temporary fix if we had some other mechanism to signal that a workspace should not show the debug buttons, potentially a new property in c_cpp_properties.json, but then if they switched to being a configuration provider then that wouldn't work (unless the configuration provider API also added a property), so I'm hesitant to do that.

It seems like the target audience for the debugShortcut is different from the target audience of users who would install the PlatformIO extension so I'm not too worried about conflicts/issues currently.

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

The change didn't work for some unknown reason. Also, I found out that language-overridable is actually a subset of "resource" scope, so I just changed it to resource scope, so I'm checking if that works instead.

@sean-mcmanus sean-mcmanus changed the title Make C_Cpp.debugShortcut language-overridable. Change C_Cpp.debugShortcut to resource scope Jun 29, 2022
@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

Don't check this in until I can verify if it works -- it may not.

@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

image

I've verified that it works -- extensions can override the default and users can change it explicitly.

@sean-mcmanus sean-mcmanus merged commit 59ea146 into main Jun 29, 2022
@sean-mcmanus sean-mcmanus deleted the seanmcm/debugShortcutLanguageOverridable branch June 29, 2022 03:57
sean-mcmanus added a commit that referenced this pull request Jun 29, 2022
Make C_Cpp.debugShortcut resource scope.
@sean-mcmanus
Copy link
Copy Markdown
Contributor Author

sean-mcmanus commented Jun 29, 2022

@bobbrow Also, extensions can already override our default settings even with the pre-existing "application" scope -- VS Code just shows it's not allowed in the package.json (not sure what effect that has though) and users can only override it in the application scope, so this change just got rid of the warning shown in the package.json for extension developers and enables users to change it on a per-workspace (folder) basis.

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants