Fix new suppress terminal setting - #5463
Andy Jordan (andyleejordan) merged 1 commit into
Conversation
Sorry, something went wrong.
Behaviorally it was working, but it wasn't showing up in the settings. We also hadn't mapped it into our (pointless IMHO) `Settings` class, or put it under the "correct" section of `integratedConsole`. To be honest, it's open work to migrate away from `getSettings()` and to just using the VS Code APIs directly, but we shouldn't do it now.
|
Updates #5380. Surprised Copilot didn't point out the issue. |
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes the visibility and wiring of the “suppress terminal stopped notification” setting by properly mapping it into the extension’s Settings model and aligning the setting key with the integratedConsole grouping.
Changes:
- Add suppressTerminalStoppedNotification to IntegratedConsoleSettings so it’s represented in getSettings().
- Update runtime read/write paths to use integratedConsole.suppressTerminalStoppedNotification.
- Rename the contributed VS Code setting ID to powershell.integratedConsole.suppressTerminalStoppedNotification so it appears in the intended settings section.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/settings.ts | Adds the missing integrated console setting field to the settings model. |
| src/session.ts | Switches notification suppression logic and “Don’t Show Again” persistence to the integratedConsole setting path. |
| package.json | Renames the contributed setting ID to the integratedConsole namespace so it shows under the correct grouping. |
Sorry, something went wrong.
| if ( | ||
| getSettings().integratedConsole.suppressTerminalStoppedNotification | ||
| ) { | ||
| return; | ||
| } | ||
|
|
Copilot
AI
Apr 9, 2026
There was a problem hiding this comment.
This change drops support for the previous setting key (powershell.suppressTerminalStoppedNotification). Users who already set the old key will start seeing notifications again after updating. Consider adding a backward-compatible fallback (read both old + new keys), and/or a one-time migration that copies the old value into integratedConsole.suppressTerminalStoppedNotification.
| if ( | |
| getSettings().integratedConsole.suppressTerminalStoppedNotification | |
| ) { | |
| return; | |
| } | |
| const suppressTerminalStoppedNotification = | |
| getSettings().integratedConsole | |
| .suppressTerminalStoppedNotification; | |
| const legacySuppressTerminalStoppedNotification = vscode.workspace | |
| .getConfiguration("powershell") | |
| .get<boolean>("suppressTerminalStoppedNotification"); | |
| if ( | |
| !suppressTerminalStoppedNotification && | |
| legacySuppressTerminalStoppedNotification | |
| ) { | |
| await changeSetting( | |
| "integratedConsole.suppressTerminalStoppedNotification", | |
| true, | |
| true, | |
| this.logger, | |
| ); | |
| return; | |
| } | |
| if ( | |
| suppressTerminalStoppedNotification || | |
| legacySuppressTerminalStoppedNotification | |
| ) { | |
| return; | |
| } |
Sorry, something went wrong.
Behaviorally it was working, but it wasn't showing up in the settings. We also hadn't mapped it into our (pointless IMHO) Settings class, or put it under the "correct" section of integratedConsole.
To be honest, it's open work to migrate away from getSettings() and to just using the VS Code APIs directly, but we shouldn't do it now.