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

fix: improve terminal marker placements on windows #209136

Merged
merged 20 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
83b639d
fix: improve terminal marker placements on windows
cpendery Mar 29, 2024
7b5f74d
Merge branch 'main' into fix/improve-marker-placements
cpendery Mar 29, 2024
021b605
Merge branch 'main' into fix/improve-marker-placements
Tyriar Apr 1, 2024
b86de5e
Merge branch 'microsoft:main' into fix/improve-marker-placements
cpendery Apr 16, 2024
2cd242c
fix: use prompt height to support multiline prompt adjustments
cpendery Apr 16, 2024
69995fe
fix: only adjust input if prompt area is unwrapped
cpendery Apr 17, 2024
b662045
fix: avoid fallthrough to markers
cpendery Apr 17, 2024
fe5152a
fix: protect cloning from falling out of bounds
cpendery Apr 17, 2024
ce71a91
fix: ensure cloneMarker stays within the terminal bottom bounds
cpendery Apr 17, 2024
701cb6a
Merge branch 'main' into fix/improve-marker-placements
cpendery Apr 17, 2024
33064ec
style: drop unused import
cpendery Apr 17, 2024
d457bdc
Merge branch 'main' into fix/improve-marker-placements
cpendery Apr 17, 2024
1b0816d
Merge branch 'main' into fix/improve-marker-placements
cpendery Apr 17, 2024
c524eef
fix: starship prompt issues
cpendery Apr 17, 2024
dec68f6
fix: prompt height calculations
cpendery Apr 17, 2024
03c0c29
Merge branch 'main' into fix/improve-marker-placements
cpendery Apr 17, 2024
64a1395
Merge branch 'main' into fix/improve-marker-placements
cpendery Apr 19, 2024
744d0d8
Merge branch 'main' into fix/improve-marker-placements
Tyriar Apr 19, 2024
27a0f27
Fix encoding of escape character in pwsh
Tyriar Apr 19, 2024
3864790
Explain what handleInput is doing
Tyriar Apr 19, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ export interface ICommandDetectionCapability {
readonly onCurrentCommandInvalidated: Event<ICommandInvalidationRequest>;
setContinuationPrompt(value: string): void;
setCwd(value: string): void;
setPromptHeight(value: number): void;
setIsWindowsPty(value: boolean): void;
setIsCommandStorageDisabled(): void;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ export class TerminalCommand implements ITerminalCommand {

export interface ICurrentPartialCommand {
promptStartMarker?: IMarker;
promptHeight?: number;

commandStartMarker?: IMarker;
commandStartX?: number;
Expand Down Expand Up @@ -244,12 +245,23 @@ export interface ICurrentPartialCommand {
*/
isInvalid?: boolean;

/**
* Whether the command start marker has been adjusted on Windows.
*/
isAdjusted?: boolean;

/**
* Whether the command start marker adjustment has been attempt on new terminal input.
*/
isInputAdjusted?: boolean;

getPromptRowCount(): number;
getCommandRowCount(): number;
}

export class PartialTerminalCommand implements ICurrentPartialCommand {
promptStartMarker?: IMarker;
promptHeight?: number;

commandStartMarker?: IMarker;
commandStartX?: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
};
this._register(this._terminal.onResize(e => this._handleResize(e)));
this._register(this._terminal.onCursorMove(() => this._handleCursorMove()));
this._register(this._terminal.onData(() => this._handleInput()));
}

private _handleResize(e: { cols: number; rows: number }) {
Expand Down Expand Up @@ -207,6 +208,10 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
this._cwd = value;
}

setPromptHeight(value: number) {
this._currentCommand.promptHeight = value;
}

setIsWindowsPty(value: boolean) {
if (value && !(this._ptyHeuristics.value instanceof WindowsPtyHeuristics)) {
const that = this;
Expand Down Expand Up @@ -280,6 +285,10 @@ export class CommandDetectionCapability extends Disposable implements ICommandDe
return undefined;
}

private _handleInput(): void {
this._ptyHeuristics.value.handleInput();
}

handlePromptStart(options?: IHandleCommandOptions): void {
// Adjust the last command's finished marker when needed. The standard position for the
// finished marker `D` to appear is at the same position as the following prompt started
Expand Down Expand Up @@ -499,6 +508,8 @@ class UnixPtyHeuristics extends Disposable {
}));
}

handleInput() { }

handleCommandStart(options?: IHandleCommandOptions) {
this._hooks.commitCommandFinished();

Expand Down Expand Up @@ -652,6 +663,25 @@ class WindowsPtyHeuristics extends Disposable {
}
}

handleInput() {
cpendery marked this conversation as resolved.
Show resolved Hide resolved
const currentY = this._terminal.buffer.active.baseY + this._terminal.buffer.active.cursorY;

const hasWrappingInPrompt = Array.from({ length: (this._capability.currentCommand.promptHeight ?? 0) + 1 }, (_, i) => currentY - i).find(y => this._terminal.buffer.active.getLine(y)?.isWrapped) !== undefined;
const hasActiveCommand = this._capability.currentCommand.commandStartX !== undefined && this._capability.currentCommand.commandExecutedX === undefined;
const hasAdjusted = this._capability.currentCommand.isAdjusted === true || this._capability.currentCommand.isInputAdjusted === true;

if (!hasActiveCommand || hasAdjusted || hasWrappingInPrompt) {
return;
}
this._capability.currentCommand.isInputAdjusted = true;
this._logService.debug('CommandDetectionCapability#handleInput attempting start marker adjustment');

this._tryAdjustCommandStartMarkerScannedLineCount = 0;
this._tryAdjustCommandStartMarkerPollCount = 0;
this._tryAdjustCommandStartMarkerScheduler = new RunOnceScheduler(() => this._tryAdjustCommandStartMarker(this._terminal.registerMarker(0)!), AdjustCommandStartMarkerConstants.Interval);
this._tryAdjustCommandStartMarkerScheduler.schedule();
}

handleCommandStart() {
this._capability.currentCommand.commandStartX = this._terminal.buffer.active.cursorX;

Expand Down Expand Up @@ -713,21 +743,24 @@ class WindowsPtyHeuristics extends Disposable {
if (prompt) {
const adjustedPrompt = typeof prompt === 'string' ? prompt : prompt.prompt;
this._capability.currentCommand.commandStartMarker = this._terminal.registerMarker(0)!;
if (typeof prompt === 'object' && prompt.likelySingleLine) {
this._logService.debug('CommandDetectionCapability#_tryAdjustCommandStartMarker adjusted promptStart', `${this._capability.currentCommand.promptStartMarker?.line} -> ${this._capability.currentCommand.commandStartMarker.line}`);
this._capability.currentCommand.promptStartMarker?.dispose();
this._capability.currentCommand.promptStartMarker = cloneMarker(this._terminal, this._capability.currentCommand.commandStartMarker);
// Adjust the last command if it's not in the same position as the following
// prompt start marker
const lastCommand = this._capability.commands.at(-1);
if (lastCommand && this._capability.currentCommand.commandStartMarker.line !== lastCommand.endMarker?.line) {
lastCommand.endMarker?.dispose();
lastCommand.endMarker = cloneMarker(this._terminal, this._capability.currentCommand.commandStartMarker);
}

cpendery marked this conversation as resolved.
Show resolved Hide resolved
// Adjust the prompt start marker to the command start marker
this._logService.debug('CommandDetectionCapability#_tryAdjustCommandStartMarker adjusted promptStart', `${this._capability.currentCommand.promptStartMarker?.line} -> ${this._capability.currentCommand.commandStartMarker.line}`);
this._capability.currentCommand.promptStartMarker?.dispose();
this._capability.currentCommand.promptStartMarker = cloneMarker(this._terminal, this._capability.currentCommand.commandStartMarker, -((this._capability.currentCommand.promptHeight ?? 1) - 1));

// Adjust the last command if it's not in the same position as the following
// prompt start marker
const lastCommand = this._capability.commands.at(-1);
if (lastCommand && this._capability.currentCommand.commandStartMarker.line !== lastCommand.endMarker?.line) {
lastCommand.endMarker?.dispose();
lastCommand.endMarker = cloneMarker(this._terminal, this._capability.currentCommand.commandStartMarker, -((this._capability.currentCommand.promptHeight ?? 1) - 1));
}

// use the regex to set the position as it's possible input has occurred
this._capability.currentCommand.commandStartX = adjustedPrompt.length;
this._logService.debug('CommandDetectionCapability#_tryAdjustCommandStartMarker adjusted commandStart', `${start.line} -> ${this._capability.currentCommand.commandStartMarker.line}:${this._capability.currentCommand.commandStartX}`);
this._capability.currentCommand.isAdjusted = true;
this._flushPendingHandleCommandStartTask();
return;
}
Expand Down Expand Up @@ -1049,5 +1082,7 @@ function getXtermLineContent(buffer: IBuffer, lineStart: number, lineEnd: number
}

function cloneMarker(xterm: Terminal, marker: IXtermMarker, offset: number = 0): IXtermMarker | undefined {
return xterm.registerMarker(marker.line - (xterm.buffer.active.baseY + xterm.buffer.active.cursorY) + offset);
const cursorY = xterm.buffer.active.baseY + xterm.buffer.active.cursorY;
const cursorYOffset = marker.line - cursorY + offset;
return xterm.registerMarker((cursorY + cursorYOffset) < 0 ? -cursorY : cursorYOffset);
}
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,10 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati
this.capabilities.get(TerminalCapability.CommandDetection)?.setIsCommandStorageDisabled();
return true;
}
case 'PromptHeight': {
this.capabilities.get(TerminalCapability.CommandDetection)?.setPromptHeight(parseInt(value));
return true;
}
}
}
case VSCodeOscPt.SetMark: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ __vsc_update_cwd() {
builtin printf '\e]633;P;Cwd=%s\a' "$(__vsc_escape_value "$__vsc_cwd")"
}

__vsc_update_prompt_height() {
__vsc_prompt_height="$(("$(builtin printf "%s" "${PS1@P}" | wc -l)" + 1))"
builtin printf '\e]633;P;PromptHeight=%s\a' "$(__vsc_escape_value "$__vsc_prompt_height")"
}
cpendery marked this conversation as resolved.
Show resolved Hide resolved

__vsc_command_output_start() {
builtin printf '\e]633;E;%s;%s\a' "$(__vsc_escape_value "${__vsc_current_command}")" $__vsc_nonce
builtin printf '\e]633;C\a'
Expand Down Expand Up @@ -229,6 +234,7 @@ __vsc_precmd() {
__vsc_command_complete "$__vsc_status"
__vsc_current_command=""
__vsc_update_prompt
__vsc_update_prompt_height
__vsc_first_prompt=1
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,12 @@ if ($env:VSCODE_ENV_APPEND) {
function Global:__VSCode-Escape-Value([string]$value) {
# NOTE: In PowerShell v6.1+, this can be written `$value -replace '…', { … }` instead of `[regex]::Replace`.
# Replace any non-alphanumeric characters.
$Result = [regex]::Replace($value, '[\\\n;]', { param($match)
[regex]::Replace($value, "[$([char]0x1b)\\\n;]", { param($match)
# Encode the (ascii) matches as `\x<hex>`
-Join (
[System.Text.Encoding]::UTF8.GetBytes($match.Value) | ForEach-Object { '\x{0:x2}' -f $_ }
)
})
# `e is only availabel in pwsh 6+
if ($PSVersionTable.PSVersion.Major -lt 6) {
$Result = $Result -replace "`e", '\x1b'
}
$Result
}

function Global:Prompt() {
Expand Down Expand Up @@ -95,7 +90,13 @@ function Global:Prompt() {
Write-Error "failure" -ea ignore
}
# Run the original prompt
$Result += $Global:__VSCodeOriginalPrompt.Invoke()
$OriginalPrompt += $Global:__VSCodeOriginalPrompt.Invoke()
$Result += $OriginalPrompt

# Prompt height
# OSC 633 ; <Property>=<Value> ST
$Result += "$([char]0x1b)]633;P;PromptHeight=$(__VSCode-Escape-Value ($OriginalPrompt -Split '\n').Count)`a"

# Write command started
$Result += "$([char]0x1b)]633;B`a"
$Global:__LastHistoryId = $LastHistoryEntry.Id
Expand Down
Loading