Skip to content

Added option whether to show terminal reuse alert#44461

Merged
dbaeumer merged 6 commits intomicrosoft:masterfrom
oriash93:oriash93/42993
Jun 7, 2018
Merged

Added option whether to show terminal reuse alert#44461
dbaeumer merged 6 commits intomicrosoft:masterfrom
oriash93:oriash93/42993

Conversation

@oriash93
Copy link
Contributor

@oriash93 oriash93 commented Feb 27, 2018

Allow user to disable "Terminal will be reused by tasks, press any key to close it" message.

fixes #42993

@msftclas
Copy link

msftclas commented Feb 27, 2018 •
edited
Loading

CLA assistant check
All CLA requirements met.

Tyriar
Tyriar reviewed Feb 27, 2018
View reviewed changes
src/vs/workbench/parts/terminal/electron-browser/terminal.contribution.ts Outdated
Copy link
Member

@Tyriar Tyriar Feb 27, 2018

Choose a reason for hiding this comment

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

The terminal.integrated namespace currently doesn't have anything related to tasks/debug in it AFAIK. @dbaeumer is there a tasks settings namespace?

Copy link
Member

@dbaeumer dbaeumer Feb 28, 2018

Choose a reason for hiding this comment

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

Yes, everything in a tasks.json is basically a setting. So whether the message is shown or not should be a task settings and a global setting in the tasks.json.

The corresponding schema for this is here: https://github.com/Microsoft/vscode/blob/master/src\vs\workbench\parts\tasks\electron-browser\jsonSchema_v2.ts#L7

@dbaeumer dbaeumer self-requested a review February 28, 2018 10:24
@dbaeumer dbaeumer assigned dbaeumer and unassigned Tyriar Feb 28, 2018
@oriash93
Copy link
Contributor Author

oriash93 commented Mar 1, 2018

I hope I got it right this time.

@dbaeumer dbaeumer added this to the March 2018 milestone Mar 1, 2018
@dbaeumer
Copy link
Member

dbaeumer commented Mar 14, 2018

I looked at the PR and it goes into the right direction. Since tasks have already a presentation property I opt to include it there and not make it a first level attribute. Something:

"presentation": {
    ....
    "reuseMessage": boolean
}

The default should be true although not a falsify value. But it makes more sense then negating the key.

And you only need to adopt the V2 schema since the presentation attribute is only supported on V2.

@oriash93
Copy link
Contributor Author

oriash93 commented Mar 15, 2018

@dbaeumer, I force-pushed the whole branch, but take a look at the last commit.

@dbaeumer
Copy link
Member

dbaeumer commented Mar 16, 2018

Code looks OK to me. Can you add a test to ensure the configuration is passed correctly.

@bpasero bpasero modified the milestones: March 2018, April 2018 Apr 6, 2018
@oriash93
Copy link
Contributor Author

oriash93 commented Apr 7, 2018

@dbaeumer I did the same as with "focus" property in configuration.test.ts.

@dbaeumer
Copy link
Member

dbaeumer commented Apr 24, 2018

@oriash93 I need to push this to Mai. I was extremely busy with other things in April. Sorry for that.

@dbaeumer dbaeumer modified the milestones: April 2018, May 2018 Apr 24, 2018
@dbaeumer
Copy link
Member

dbaeumer commented May 30, 2018

Sorry for moving it again. I will promise to look into it first thing in June.

@dbaeumer dbaeumer modified the milestones: May 2018, June 2018 May 30, 2018
@dbaeumer dbaeumer merged commit f3966af into microsoft:master Jun 7, 2018
@dbaeumer
Copy link
Member

dbaeumer commented Jun 7, 2018

I merged in the PR however I will tweak it a little. Mostly rename reuseMessage to showReuseMessage.

dbaeumer added a commit that referenced this pull request Jun 7, 2018
@dbaeumer
Some cleanup for PR: #44461
349d29d
@oriash93 oriash93 deleted the oriash93/42993 branch June 7, 2018 15:02
@gwk
Copy link

gwk commented Jun 9, 2018

Is this available in the latest insiders build? I can't seem to get it to work but maybe I'm missing something.

@dbaeumer
Copy link
Member

dbaeumer commented Jun 11, 2018

I should be in today's insider.

@buggymcbugfix
Copy link

buggymcbugfix commented Mar 31, 2019

Has this not landed in the current release version yet? I'm getting "Property message is not allowed" for "reuseMessage".

@dbaeumer
Copy link
Member

dbaeumer commented Apr 1, 2019

The property name is "showReuseMessage"

@umrashrf
Copy link

umrashrf commented May 22, 2019 

{
    // See https://go.microsoft.com/fwlink/?LinkId=733558
    // for the documentation about the tasks.json format
    "version": "2.0.0",
    "tasks": [
        {
            "label": "clean",
            "type": "shell",
            "windows": {
                "options": {
                    "shell": {
                        "executable": "cmd.exe",
                        "args": [
                            "/d", "/c"
                        ]
                    }
                },
                "command": "del /S /Q *.pyc *.log"
            },
            "presentation": {
                "showReuseMessage": false,
            }
        }
    ]
}

@rustyx
Copy link

rustyx commented Jul 12, 2019

@dbaeumer so this just hides the message "Press any key to close the terminal.", but user still has to press a key to reveal their terminal??

@dbaeumer
Copy link
Member

dbaeumer commented Aug 5, 2019

@rustyx you can still set presentation.reveal to 'never' which will not bring the terminal to the front.

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Tyriar Tyriar Tyriar left review comments

@dbaeumer dbaeumer Awaiting requested review from dbaeumer

Assignees

@dbaeumer dbaeumer

Labels

None yet

Projects

None yet

Milestone

June 2018

Development

Successfully merging this pull request may close these issues.

Allow user to disable "Terminal will be reused by tasks, press any key to close it" message.

9 participants

@oriash93 @msftclas @dbaeumer @gwk @buggymcbugfix @umrashrf @rustyx @Tyriar @bpasero