Skip to content

Fix new suppress terminal setting#5463

Merged
andyleejordan merged 1 commit intomainfrom
fix-setting
Apr 9, 2026
Merged

Fix new suppress terminal setting#5463
andyleejordan merged 1 commit intomainfrom
fix-setting

Conversation

@andyleejordan
Copy link
Copy Markdown
Member

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.

Copilot AI review requested due to automatic review settings April 9, 2026 16:56
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.
@andyleejordan
Copy link
Copy Markdown
Member Author

Updates #5380. Surprised Copilot didn't point out the issue.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +1162 to 1167
if (
getSettings().integratedConsole.suppressTerminalStoppedNotification
) {
return;
}

Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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;
}

Copilot uses AI. Check for mistakes.
@andyleejordan andyleejordan merged commit 01e8ef8 into main Apr 9, 2026
8 checks passed
@andyleejordan andyleejordan deleted the fix-setting branch April 9, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants