-
Notifications
You must be signed in to change notification settings - Fork 30k
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
add option to disable terminal exit alert #40861
add option to disable terminal exit alert #40861
Conversation
@@ -687,7 +689,8 @@ export class TerminalInstance implements ITerminalInstance { | |||
this._isExiting = true; | |||
this._process = null; | |||
let exitCodeMessage: string; | |||
if (exitCode) { | |||
|
|||
if (exitCode && this.configurationService.getValue('terminal.integrated.showExitAlert')) { |
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.
Currently all interactions with the config service is managed within TerminalConfigHelper
, could you change to use this._configHelper.config.showExitAlert
instead? You'll also need to add it to the interface defined in terminal.ts
@@ -687,7 +687,8 @@ export class TerminalInstance implements ITerminalInstance { | |||
this._isExiting = true; | |||
this._process = null; | |||
let exitCodeMessage: string; | |||
if (exitCode) { | |||
|
|||
if (exitCode && this._configHelper.config.showExitAlert) { |
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.
Looking at this again I think the showExitAlert
needs to be added to this line at the bottom of the function:
this._messageService.show(Severity.Error, exitCodeMessage)
exitCodeMessage
is shared between both the message box and also tasks which will be output to the terminal if it fails. In the current state when tasks fail an empty line will be added to the terminal when the task fails instead of the message.
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.
Sure! Update.
…0503-add-option-to-disable-terminal-exit-alert
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.
Thanks 😃
Fixes #40503