Skip to content

Handle clearTerminal message by using vscode clear command #2316

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 27, 2019
Merged

Handle clearTerminal message by using vscode clear command #2316

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4ef11c
handle clearTerminal message by using vscode clear command
TylerLeonhardt Nov 19, 2019
0efeb46
Codacy
TylerLeonhardt Nov 19, 2019
05056bb
changed name of setting
TylerLeonhardt Nov 21, 2019
62fff5b
delete comment
TylerLeonhardt Nov 21, 2019
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
4 changes: 4 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,10 @@
"default": false,
"description": "Falls back to the legacy (lightweight) ReadLine experience. This will disable the use of PSReadLine in the PowerShell Integrated Console."
},
"powershell.integratedConsole.useTrueClear": {
Copy link
Contributor

@rjmholt rjmholt Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

How about something like powershell.integrateConsole.clearScrollbackBuffer. Or slightly more futureproof maybe: "powershell.integratedConsole.clearAction": "scrollback" | "clear". Something with the term scrollback in it, just since TrueClear sounds a bit like a background check or a bad liquor to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

LOL

Ok. I kinda like the clearAction one. Let's go with that

Copy link
Member Author

Choose a reason for hiding this comment

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

went with forceClearScrollbackBuffer because we can't change the behavior of clear-host other than forcing a full clear.

"type": "boolean",
"description": "Use the vscode API to clear the terminal since that's the only way to trigger a true clear. A true clear is a clear that shows nothing when you scroll up after running Clear-Host."
},
"powershell.debugging.createTemporaryIntegratedConsole": {
"type": "boolean",
"default": false,
Expand Down
17 changes: 16 additions & 1 deletion src/features/ExtensionCommands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import * as fs from "fs";
import * as os from "os";
import * as path from "path";
import * as vscode from "vscode";
import { LanguageClient, NotificationType, Position, Range, RequestType } from "vscode-languageclient";
import { LanguageClient, NotificationType, NotificationType0,
Position, Range, RequestType } from "vscode-languageclient";
import { IFeature } from "../feature";
import { Logger } from "../logging";
import Settings = require("../settings");

export interface IExtensionCommand {
name: string;
Expand Down Expand Up @@ -155,6 +157,9 @@ export const SetStatusBarMessageRequestType =
new RequestType<IStatusBarMessageDetails, EditorOperationResponse, void, void>(
"editor/setStatusBarMessage");

export const ClearTerminalNotificationType =
new NotificationType0("editor/clearTerminal");

export interface ISaveFileDetails {
filePath: string;
newPath?: string;
Expand Down Expand Up @@ -265,6 +270,16 @@ export class ExtensionCommandsFeature implements IFeature {
this.languageClient.onRequest(
SetStatusBarMessageRequestType,
(messageDetails) => this.setStatusBarMessage(messageDetails));

this.languageClient.onNotification(
ClearTerminalNotificationType,
() => {
// We check to see if they have TrueClear on. If not, no-op because the
// overriden Clear-Host already calls [System.Console]::Clear()
Copy link
Contributor

Choose a reason for hiding this comment

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

If Clear just triggered \x1b[3J it would work everywhere as that clears scrollback.

https://github.com/xtermjs/xterm.js/blob/70babeacb62fe05264d64324ca1f4436997efa1b/src/InputHandler.ts#L908-L918

Copy link
Member Author

Choose a reason for hiding this comment

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

So glad I cc'd you on that tweet - this is so much better

Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't seem to work for me on Windows. Is support for the 3 option new? \x1b[2J works for me (in that it clears the current display) but \x1b[3J appears to be ignored.

Copy link
Contributor

@Tyriar Tyriar Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

On Windows the compatibility layer might filter it out or something. I would expect this fix to be in the form of changing how clear acts in conpty to send ED 3 and 2 (i think) to clear the viewport and scrollback when Console.Clear is triggered. Maybe this is that issue? microsoft/terminal#2260

Copy link
Member Author

@TylerLeonhardt TylerLeonhardt Nov 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@SeeminglyScience were you trying:

Write-Host "`e[3J"

Copy link
Member Author

Choose a reason for hiding this comment

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

damn ok - thought that maybe this would be viable but we'll probably have to do what this PR does and use the vscode API.

Copy link
Member Author

Choose a reason for hiding this comment

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

@SeeminglyScience one more thing:

$([char]27)

Copy link
Collaborator

Choose a reason for hiding this comment

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

@TylerLeonhardt What about it?

Copy link

Choose a reason for hiding this comment

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

I got this from one of the Clear issues in the new Windows Terminal.

$host.UI.Write( "$([char]27)[3J$([char]27)[2J" )

It mostly works but requires a few tweaks. As is, the command leaves the cursor at the bottom so I expanded on it to reset the cursor to the top:

$host.UI.RawUI.CursorPosition = New-Object System.Management.Automation.Host.Coordinates 0,0
$host.UI.Write( "$([char]27)[3J$([char]27)[2J" )

It seems to work better in latest preview. Last time I tested this, I was still having to do manual Clear-Host in addition to it.

Remaining issue is the scrollbar doesn't reset so it looks like there is stuff there but there isn't. I still use VS Code's Clear to remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah VS Code's Clear is probably the most reliable here but that's great info @dragonwolf83!

if (Settings.load().integratedConsole.useTrueClear) {
vscode.commands.executeCommand("workbench.action.terminal.clear");
}
});
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/settings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export interface IIntegratedConsoleSettings {
showOnStartup?: boolean;
focusConsoleOnExecute?: boolean;
useLegacyReadLine?: boolean;
useTrueClear?: boolean;
}

export function load(): ISettings {
Expand Down Expand Up @@ -155,6 +156,8 @@ export function load(): ISettings {
showOnStartup: true,
focusConsoleOnExecute: true,
useLegacyReadLine: false,
// This behavior is expected on Windows but not on non-Windows
useTrueClear: utils.isWindowsOS(),
};

return {
Expand Down