Skip to content
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 buttons for moving the terminal around. #2704

Merged
merged 3 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 69 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,33 @@
"command": "PowerShell.InvokeRegisteredEditorCommand",
"title": "Invoke Registered Editor Command",
"category": "PowerShell"
},
{
"command": "workbench.action.closePanel",
"title": "Close panel",
"category": "PowerShell",
"icon": {
"light": "resources/light/ClosePanel.svg",
"dark": "resources/dark/ClosePanel.svg"
}
},
{
"command": "workbench.action.positionPanelLeft",
"title": "Move panel left",
"category": "PowerShell",
"icon": {
"light": "resources/light/MovePanelLeft.svg",
"dark": "resources/dark/MovePanelLeft.svg"
}
},
{
"command": "workbench.action.positionPanelBottom",
"title": "Move panel to bottom",
"category": "PowerShell",
"icon": {
"light": "resources/light/MovePanelBottom.svg",
"dark": "resources/dark/MovePanelBottom.svg"
}
}
],
"menus": {
Expand Down Expand Up @@ -306,12 +333,27 @@
],
"editor/title": [
{
"when": "editorLangId == powershell",
"when": "editorLangId == powershell && config.powershell.buttons.MovePanelToBottomButtonVisibility",
"command": "workbench.action.positionPanelBottom",
"group": "navigation@97"
},
{
"when": "editorLangId == powershell && config.powershell.buttons.MovePanelLeftButtonVisibility",
"command": "workbench.action.positionPanelLeft",
"group": "navigation@98"
},
{
"when": "editorLangId == powershell && config.powershell.buttons.ClosePanelButtonVisibility",
"command": "workbench.action.closePanel",
"group": "navigation@99"
},
{
"when": "editorLangId == powershell && config.powershell.buttons.RunButtonVisibility",
"command": "workbench.action.debug.start",
"group": "navigation@100"
},
{
"when": "editorLangId == powershell",
"when": "editorLangId == powershell && config.powershell.buttons.RunSelectionButtonVisibility",
"command": "PowerShell.RunSelection",
"group": "navigation@101"
}
Expand Down Expand Up @@ -800,6 +842,31 @@
],
"default": "Diagnostic",
"description": "Defines the verbosity of output to be used when debugging a test or a block. For Pester 5 and newer the default value Diagnostic will print additional information about discovery, skipped and filtered tests, mocking and more."
},
"powershell.buttons.RunButtonVisibility": {
"type": "boolean",
"default": true,
"description": "Show the Run button in the editor titlebar."
},
"powershell.buttons.RunSelectionButtonVisibility": {
"type": "boolean",
"default": true,
"description": "Show the Run Selection button in the editor titlebar."
},
"powershell.buttons.ClosePanelButtonVisibility": {
"type": "boolean",
"default": false,
"description": "Show the Close panel button in the editor titlebar."
},
"powershell.buttons.MovePanelLeftButtonVisibility": {
"type": "boolean",
"default": false,
"description": "Show the Move panel left button in the editor titlebar."
},
"powershell.buttons.MovePanelToBottomButtonVisibility": {
"type": "boolean",
"default": false,
"description": "Show the Move panel to bottom button in the editor titlebar."
Copy link
Member

Choose a reason for hiding this comment

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

This is really awesome and gives the user a ton of flexibility but I don't think some is going to want just 1 button or just 2 buttons from what appears to be two groups here:

powershell.buttons.showRunButtons
powershell.buttons.showPanelMovementButtons

Can you change it to these 2 settings and use these everyewhere? I discussed this with @rjmholt and @SydneyhSmith

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, it still fits my needs so I don't mind limiting the options a bit. I'm just curious why limit it if the work has already been done? I can imagine there's people out there that never use the vertical split view or always use the run selection option that might not care for those buttons.

Copy link
Member

Choose a reason for hiding this comment

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

I'm very wary to add configuration because often times people get lost in it. There have been many instances where folks just don't realize that settings already exist or find themselves digging trying to figure out the right configuration for them.

I do want there to be some way to toggle showing these buttons and that's kinda why we want 2 settings instead of 5.

I know the difference is only 3 but we try to have this mindset with all new settings.

I can imagine there's people out there that never use the vertical split view or always use the run selection option that might not care for those buttons.

Maybe. But those same people might be just as happy seeing all the buttons... in the off chance that they use the other.

It's in situations like this where I go "let's expose the least amount of stuff that makes sense (in other words 2 settings) and wait for a user to come around to ask for more."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand and I've fixed it now.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @MartinGC94!

}
}
},
Expand Down
9 changes: 9 additions & 0 deletions resources/dark/ClosePanel.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions resources/dark/MovePanelBottom.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions resources/dark/MovePanelLeft.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
9 changes: 9 additions & 0 deletions resources/light/ClosePanel.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions resources/light/MovePanelBottom.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions resources/light/MovePanelLeft.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions src/features/ISECompatibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export class ISECompatibilityFeature implements IFeature {
{ path: "powershell.integratedConsole", name: "focusConsoleOnExecute", value: false },
{ path: "files", name: "defaultLanguage", value: "powershell" },
{ path: "workbench", name: "colorTheme", value: "PowerShell ISE" },
{ path: "powershell.buttons", name: "ClosePanelButtonVisibility", value: true },
{ path: "powershell.buttons", name: "MovePanelLeftButtonVisibility", value: true },
{ path: "powershell.buttons", name: "MovePanelToBottomButtonVisibility", value: true }
];
private iseCommandRegistration: vscode.Disposable;
private defaultCommandRegistration: vscode.Disposable;
Expand Down
19 changes: 19 additions & 0 deletions src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ export interface ISettings {
bugReporting?: IBugReportingSettings;
sideBar?: ISideBarSettings;
pester?: IPesterSettings;
buttons?: IButtonSettings;
}

export interface IStartAsLoginShellSettings {
Expand All @@ -125,6 +126,14 @@ export interface IPesterSettings {
debugOutputVerbosity?: string;
}

export interface IButtonSettings {
RunButtonVisibility?: boolean;
RunSelectionButtonVisibility?: boolean;
ClosePanelButtonVisibility?: boolean;
MovePanelLeftButtonVisibility?: boolean;
MovePanelToBottomButtonVisibility?: boolean;
}

export function load(): ISettings {
const configuration: vscode.WorkspaceConfiguration =
vscode.workspace.getConfiguration(
Expand Down Expand Up @@ -192,6 +201,14 @@ export function load(): ISettings {
CommandExplorerVisibility: true,
};

const defaultButtonSettings: IButtonSettings = {
RunButtonVisibility: true,
RunSelectionButtonVisibility: true,
ClosePanelButtonVisibility: false,
MovePanelLeftButtonVisibility: false,
MovePanelToBottomButtonVisibility: false
};

const defaultPesterSettings: IPesterSettings = {
useLegacyCodeLens: true,
outputVerbosity: "FromPreference",
Expand Down Expand Up @@ -237,6 +254,8 @@ export function load(): ISettings {
configuration.get<ISideBarSettings>("sideBar", defaultSideBarSettings),
pester:
configuration.get<IPesterSettings>("pester", defaultPesterSettings),
buttons:
configuration.get<IButtonSettings>("buttons", defaultButtonSettings),
startAsLoginShell:
// tslint:disable-next-line
// We follow the same convention as VS Code - https://github.com/microsoft/vscode/blob/ff00badd955d6cfcb8eab5f25f3edc86b762f49f/src/vs/workbench/contrib/terminal/browser/terminal.contribution.ts#L105-L107
Expand Down