Fix: Add env object syntax to launch.json schema for cppdbg and cppvsdbg - #14691
Fix: Add env object syntax to launch.json schema for cppdbg and cppvsdbg#14691Prashant Kumar Rai (8prashant) wants to merge 3 commits into
env object syntax to launch.json schema for cppdbg and cppvsdbg#14691Conversation
There was a problem hiding this comment.
Pull request overview
Adds schema support for shorthand env objects in C++ launch configurations, addressing issue #12537.
Changes:
- Adds
envschemas forcppdbgandcppvsdbg. - Adds localized documentation for the syntax.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
Extension/package.json |
Defines shorthand env objects. |
Extension/package.nls.json |
Documents the new schema property. |
Suppressed comments (1)
Extension/package.json:6059
- The newly validated
envform is silently ignored when this cppvsdbg configuration is started with Run Without Debugging.debugAdapterDescriptorFactory.ts:59-61selects the inline adapter for these launch configurations, whilerunWithoutDebuggingAdapter.ts:62-80reads onlyenvironment. Please mergeconfig.envinto the process environment (and cover that path in the existing no-debug tests), or treatenvas unsupported so the normal adapter is used.
"env": {
"type": "object",
"description": "%c_cpp.debuggers.env.description%",
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
I don't think we should have two properties that do the same thing (also, this PR does not update the optionsSchema.json which means the next time package.json is generated, this will be deleted). Adding Andrew Wang (@WardenGnaw) for comment. |
|
Thanks for the contribution. This change will work for One issue with this change is that Aside from updating the OptionsSchema.json. There needs to be a change to MIEngine to support this or limit the new schema entry to The other option is to add a cpptools configuration-provider conversion from |
|
Thanks Andrew Wang (WardenGnaw) for the review. I’ve pushed updates to address this by fixing the no-debug path so the inline adapter now merges both environment and env with env taking precedence, adding cppdbg normalization in the configuration provider to convert env to environment so MIEngine receives the format it currently supports, updating OptionsSchema.json so the new env schema entries are preserved during regeneration, and adding Run Without Debugging integration coverage to verify env is applied and correctly overrides duplicate environment entries. I intentionally kept this fix in cpptools rather than making MIEngine changes in this PR because it resolves both normal launch and Run Without Debugging immediately, whereas a MIEngine-only change would not cover no-debug, and this approach avoids cross-repo dependency and release-coordination risk while keeping the change scoped and low risk; native MIEngine env support can still be added later as a separate follow-up enhancement. |
|
Hi Sean McManus (@sean-mcmanus) |
|
Prashant Kumar Rai (@8prashant) Yeah, sorry, I've been busy with other stuff and forgot about these PRs. |
| lastContents = await util.readFileText(filePath, 'utf8'); | ||
| return lastContents.trim(); |
Sean McManus (sean-mcmanus)
left a comment
There was a problem hiding this comment.
Reviewed the current head; see the inline comments.
|
|
||
| // cppdbg (MIEngine) consumes the legacy `environment` array, not `env`. | ||
| // Convert here so both syntaxes work while preserving `env` precedence. | ||
| this.resolveEnvObjectForCppdbg(config); |
There was a problem hiding this comment.
✨Copilot (agent117): [Minor] The added scenario never exercises this conversion. It starts with { noDebug: true }, which selects RunWithoutDebuggingAdapter; that adapter independently consumes env, so the test would still pass if resolveEnvObjectForCppdbg were removed. Please cover the normal cppdbg path or directly assert the provider's resolved configuration, including env precedence, so the MIEngine compatibility behavior introduced here is protected.
| console: debugType === 'cppvsdbg' ? 'internalConsole' : undefined | ||
| }, | ||
| { noDebug: true }); | ||
|
|
There was a problem hiding this comment.
✨Copilot (agent117): [Minor] This can complete before the no-debug session has terminated: envTest.cpp writes its file before exiting, while the inline adapter emits terminated only after the terminal-shell execution ends. The test neither subscribes to onDidTerminateDebugSession before launch nor stops the session on failure, so it can leave an active session for later cases; the current Linux run has both following cases timing out while waiting for debug events. Please await this session's termination and clean it up in finally.
| "{Locked=\"[ { \\\"name\\\": \\\"\"} {Locked=\"\\\", \\\"value\\\": \\\"\"} {Locked=\"\\\" } ]\"} {Locked=\"[ { \\\"\"} {Locked=\"\\\": \\\"\"} {Locked=\"\\\" } ]\"}" | ||
| ] | ||
| }, | ||
| "c_cpp.debuggers.env.description": "Object of environment variables to add to the environment for the program. Example: { \"MY_VAR\": \"value\" }. Use `environment` for the array-of-objects form.", |
There was a problem hiding this comment.
✨Copilot (agent117): [Nit] Please use the structured { message, comment } form and lock the JSON example plus the literal environment key. The adjacent c_cpp.debuggers.environment.description does this; without locks, localization can translate schema syntax or property names and leave a misleading launch.json example.
| @@ -0,0 +1,21 @@ | |||
| #include <cstdlib> | |||
There was a problem hiding this comment.
✨Copilot (agent117): [Nit] This new asset was committed with CRLF while the neighboring C++ assets use LF, so every added line is reported as trailing whitespace by git diff --check. Please normalize the file to LF.
Problem
#12537 : Using the shorthand
envobject syntax inlaunch.jsonproduces a false validation warning: