From 5527b85b48273e131916139291268039921b1132 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 13:47:36 -0500 Subject: [PATCH 01/22] try to come up with diff approach --- .../common/capabilities/capabilities.ts | 1 + .../shellEnvDetectionCapability.ts | 31 ++++++++++++++++-- .../common/xterm/shellIntegrationAddon.ts | 2 +- .../common/scripts/shellIntegration-bash.sh | 32 +++++++++---------- 4 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/capabilities.ts b/src/vs/platform/terminal/common/capabilities/capabilities.ts index ba7597b9d926d..08e234517467e 100644 --- a/src/vs/platform/terminal/common/capabilities/capabilities.ts +++ b/src/vs/platform/terminal/common/capabilities/capabilities.ts @@ -158,6 +158,7 @@ export interface IShellEnvDetectionCapability { startEnvironmentSingleVar(isTrusted: boolean): void; setEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; endEnvironmentSingleVar(isTrusted: boolean): void; + applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean; } export const enum CommandInvalidationReason { diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index be9577acfe939..06234fb71549d 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -14,7 +14,6 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv private _pendingEnv: Map | undefined; private _env: Map = new Map(); get env(): Map { return this._env; } - private readonly _onDidChangeEnv = this._register(new Emitter>()); readonly onDidChangeEnv = this._onDidChangeEnv.event; @@ -59,8 +58,34 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (!this._pendingEnv) { return; } - this._env = this._pendingEnv; + const didDiffer = this.applyEnvironmentDiff(this._pendingEnv, isTrusted); + if (didDiffer) { + this._env = this._pendingEnv; + } this._pendingEnv = undefined; - this._onDidChangeEnv.fire(this._env); + } + // Returns true if the environment differs, and was updated. + applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean { + if (!isTrusted) { + return false; + } + + let envDiffers: boolean = false; + + for (const [key, value] of env.entries()) { + if (this._env.has(key) && this._env.get(key) === value) { + // Do nothing + } else if (value !== undefined) { + this._env.set(key, value); + envDiffers = true; + } + } + + if (envDiffers) { + this._onDidChangeEnv.fire(this._env); + return true; + } + + return false; } } diff --git a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts index 92b1851c275c8..feccd25c3b0b6 100644 --- a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts +++ b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts @@ -467,7 +467,7 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati if (arg0 !== undefined) { try { const env = JSON.parse(deserializeMessage(arg0)); - this._createOrGetShellEnvDetection().setEnvironment(env, arg1 === this._nonce); + this._createOrGetShellEnvDetection().applyEnvironmentDiff(env, arg1 === this._nonce); } catch (e) { this._logService.warn('Failed to parse environment from shell integration sequence', arg0); } diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 090f40212d2eb..3040617a5a6de 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -214,16 +214,16 @@ __vsc_update_cwd() { builtin printf '\e]633;P;Cwd=%s\a' "$(__vsc_escape_value "$__vsc_cwd")" } -# __vsc_update_env() { -# builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce -# for var in $(compgen -v); do -# if printenv "$var" >/dev/null 2>&1; then -# value=$(builtin printf '%s' "${!var}") -# builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce -# fi -# done -# builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce -# } +__vsc_update_env() { + builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce + for var in $(compgen -v); do + if printenv "$var" >/dev/null 2>&1; then + value=$(builtin printf '%s' "${!var}") + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce + fi + done + builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce +} __vsc_command_output_start() { if [[ -z "${__vsc_first_prompt-}" ]]; then @@ -252,9 +252,9 @@ __vsc_command_complete() { fi __vsc_update_cwd - # if [ "$__vsc_stable" = "0" ]; then - # __vsc_update_env - # fi + if [ "$__vsc_stable" = "0" ]; then + __vsc_update_env + fi } __vsc_update_prompt() { # in command execution @@ -285,9 +285,9 @@ __vsc_precmd() { __vsc_first_prompt=1 __vsc_update_prompt - # if [ "$__vsc_stable" = "0" ]; then - # __vsc_update_env - # fi + if [ "$__vsc_stable" = "0" ]; then + __vsc_update_env + fi } __vsc_preexec() { From 960aedaa0f9fd900291273629528c991e2d7716c Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 14:31:25 -0500 Subject: [PATCH 02/22] add fasher bash code --- .../common/scripts/shellIntegration-bash.sh | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 3040617a5a6de..a6f5e1262f262 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -216,12 +216,27 @@ __vsc_update_cwd() { __vsc_update_env() { builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce - for var in $(compgen -v); do - if printenv "$var" >/dev/null 2>&1; then - value=$(builtin printf '%s' "${!var}") + # for var in $(compgen -v); do + # if printenv "$var" >/dev/null 2>&1; then + # value=$(builtin printf '%s' "${!var}") + # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce + # fi + # done + + ## Faster than above + # for var in $(compgen -v); do + # value="${!var}" + # if [ -n "$value" ]; then + # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce + # fi + # done + + ## Faster than above + while IFS='=' read -r var value; do + if [ -n "$value" ]; then builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce fi - done + done < <(env) builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce } From 290e0cdd2cd21a40793c145344f821c96bfe5de7 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 14:53:09 -0500 Subject: [PATCH 03/22] use grep --- .../common/scripts/shellIntegration-bash.sh | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index a6f5e1262f262..48b10b7388f96 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -232,11 +232,17 @@ __vsc_update_env() { # done ## Faster than above - while IFS='=' read -r var value; do - if [ -n "$value" ]; then - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce - fi - done < <(env) + # while IFS='=' read -r var value; do + # if [ -n "$value" ]; then + # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce + # fi + # done < <(env) + + ## Slighly faster than above + env | grep '=' | while IFS='=' read -r var value; do + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + done + builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce } From d3ef701f3f34248d2e3904b1ba6aeeaa0554d184 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 16:17:34 -0500 Subject: [PATCH 04/22] instant bash is back! --- .../common/extHostTerminalShellIntegration.ts | 48 +++++++-------- .../common/scripts/shellIntegration-bash.sh | 58 ++++++++++--------- 2 files changed, 56 insertions(+), 50 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts index 076f1b725cc36..7afc0eabe5f76 100644 --- a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts +++ b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts @@ -55,30 +55,30 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH })); // Convenient test code: - // this.onDidChangeTerminalShellIntegration(e => { - // console.log('*** onDidChangeTerminalShellIntegration', e); - // }); - // this.onDidStartTerminalShellExecution(async e => { - // console.log('*** onDidStartTerminalShellExecution', e); - // // new Promise(r => { - // // (async () => { - // // for await (const d of e.execution.read()) { - // // console.log('data2', d); - // // } - // // })(); - // // }); - // for await (const d of e.execution.read()) { - // console.log('data', d); - // } - // }); - // this.onDidEndTerminalShellExecution(e => { - // console.log('*** onDidEndTerminalShellExecution', e); - // }); - // setTimeout(() => { - // console.log('before executeCommand(\"echo hello\")'); - // Array.from(this._activeShellIntegrations.values())[0].value.executeCommand('echo hello'); - // console.log('after executeCommand(\"echo hello\")'); - // }, 4000); + this.onDidChangeTerminalShellIntegration(e => { + console.log('*** onDidChangeTerminalShellIntegration', e); + }); + this.onDidStartTerminalShellExecution(async e => { + console.log('*** onDidStartTerminalShellExecution', e); + // new Promise(r => { + // (async () => { + // for await (const d of e.execution.read()) { + // console.log('data2', d); + // } + // })(); + // }); + for await (const d of e.execution.read()) { + console.log('data', d); + } + }); + this.onDidEndTerminalShellExecution(e => { + console.log('*** onDidEndTerminalShellExecution', e); + }); + setTimeout(() => { + console.log('before executeCommand(\"echo hello\")'); + Array.from(this._activeShellIntegrations.values())[0].value.executeCommand('echo hello'); + console.log('after executeCommand(\"echo hello\")'); + }, 4000); } public $shellIntegrationChange(instanceId: number): void { diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 48b10b7388f96..6c90f5cba7c44 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -10,6 +10,9 @@ fi VSCODE_SHELL_INTEGRATION=1 +vsc_env_keys=() +vsc_env_values=() + # Run relevant rc/profile only if shell integration has been injected, not when run manually if [ "$VSCODE_INJECTION" == "1" ]; then if [ -z "$VSCODE_SHELL_LOGIN" ]; then @@ -214,34 +217,37 @@ __vsc_update_cwd() { builtin printf '\e]633;P;Cwd=%s\a' "$(__vsc_escape_value "$__vsc_cwd")" } + +updateEnvCache() { + local key="$1" + local value="$2" + local updated=false + + for i in "${!vsc_env_keys[@]}"; do + if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then + if [[ "${vsc_env_values[$i]}" != "$value" ]]; then + vsc_env_values[$i]="$value" + updated=true + # printf "Updated value for %s\n" "$key" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + fi + return + fi + done + + vsc_env_keys+=("$key") + vsc_env_values+=("$value") + updated=true + # printf "Added new variable %s\n" "$key" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" +} + __vsc_update_env() { builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce - # for var in $(compgen -v); do - # if printenv "$var" >/dev/null 2>&1; then - # value=$(builtin printf '%s' "${!var}") - # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce - # fi - # done - - ## Faster than above - # for var in $(compgen -v); do - # value="${!var}" - # if [ -n "$value" ]; then - # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce - # fi - # done - - ## Faster than above - # while IFS='=' read -r var value; do - # if [ -n "$value" ]; then - # builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" $__vsc_nonce - # fi - # done < <(env) - - ## Slighly faster than above - env | grep '=' | while IFS='=' read -r var value; do - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$var" "$(__vsc_escape_value "$value")" "$__vsc_nonce" - done + + while IFS='=' read -r key value; do + updateEnvCache "$key" "$value" + done < <(env) builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce } From fbb47d6aceba0edf62ad788694a90dc6d3d9a372 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 16:21:01 -0500 Subject: [PATCH 05/22] format things better --- .../workbench/api/common/extHostTerminalShellIntegration.ts | 2 +- .../terminal/common/scripts/shellIntegration-bash.sh | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts index 7afc0eabe5f76..5cbb8edcb6eb6 100644 --- a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts +++ b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts @@ -56,7 +56,7 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH // Convenient test code: this.onDidChangeTerminalShellIntegration(e => { - console.log('*** onDidChangeTerminalShellIntegration', e); + console.log('*** onDidChangeTerminalShellIntegration', e.shellIntegration.env); }); this.onDidStartTerminalShellExecution(async e => { console.log('*** onDidStartTerminalShellExecution', e); diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 6c90f5cba7c44..89128fa0d56cc 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -245,9 +245,9 @@ updateEnvCache() { __vsc_update_env() { builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce - while IFS='=' read -r key value; do - updateEnvCache "$key" "$value" - done < <(env) + while IFS='=' read -r key value; do + updateEnvCache "$key" "$value" + done < <(env) builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce } From 9ffe2d1a2df5636049d1c99582fd00c1ee622417 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 17:44:31 -0500 Subject: [PATCH 06/22] show perf in seconds --- .../common/scripts/shellIntegration-bash.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 89128fa0d56cc..4abf02fc39ebc 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -221,13 +221,11 @@ __vsc_update_cwd() { updateEnvCache() { local key="$1" local value="$2" - local updated=false for i in "${!vsc_env_keys[@]}"; do if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then if [[ "${vsc_env_values[$i]}" != "$value" ]]; then vsc_env_values[$i]="$value" - updated=true # printf "Updated value for %s\n" "$key" builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" fi @@ -237,12 +235,15 @@ updateEnvCache() { vsc_env_keys+=("$key") vsc_env_values+=("$value") - updated=true # printf "Added new variable %s\n" "$key" builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" } __vsc_update_env() { + + start_time=$(date +%s) + builtin printf "Start time: %s\n" "$start_time" + builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce while IFS='=' read -r key value; do @@ -250,6 +251,12 @@ __vsc_update_env() { done < <(env) builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce + + end_time=$(date +%s) + elapsed_time=$(($end_time - $start_time)) + + builtin printf "Time taken: $elapsed_time seconds\n" + } __vsc_command_output_start() { From f6863cb9e53d01f326b604f4f30cee183f71f8a6 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 18:10:40 -0500 Subject: [PATCH 07/22] bring back TS side - TS side has no impact on perf --- .../common/capabilities/capabilities.ts | 1 - .../shellEnvDetectionCapability.ts | 30 ++----------------- .../common/xterm/shellIntegrationAddon.ts | 2 +- 3 files changed, 3 insertions(+), 30 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/capabilities.ts b/src/vs/platform/terminal/common/capabilities/capabilities.ts index 08e234517467e..ba7597b9d926d 100644 --- a/src/vs/platform/terminal/common/capabilities/capabilities.ts +++ b/src/vs/platform/terminal/common/capabilities/capabilities.ts @@ -158,7 +158,6 @@ export interface IShellEnvDetectionCapability { startEnvironmentSingleVar(isTrusted: boolean): void; setEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; endEnvironmentSingleVar(isTrusted: boolean): void; - applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean; } export const enum CommandInvalidationReason { diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 06234fb71549d..1a3e37ad8941c 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -58,34 +58,8 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (!this._pendingEnv) { return; } - const didDiffer = this.applyEnvironmentDiff(this._pendingEnv, isTrusted); - if (didDiffer) { - this._env = this._pendingEnv; - } + this._env = this._pendingEnv; this._pendingEnv = undefined; - } - // Returns true if the environment differs, and was updated. - applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean { - if (!isTrusted) { - return false; - } - - let envDiffers: boolean = false; - - for (const [key, value] of env.entries()) { - if (this._env.has(key) && this._env.get(key) === value) { - // Do nothing - } else if (value !== undefined) { - this._env.set(key, value); - envDiffers = true; - } - } - - if (envDiffers) { - this._onDidChangeEnv.fire(this._env); - return true; - } - - return false; + this._onDidChangeEnv.fire(this._env); } } diff --git a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts index feccd25c3b0b6..92b1851c275c8 100644 --- a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts +++ b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts @@ -467,7 +467,7 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati if (arg0 !== undefined) { try { const env = JSON.parse(deserializeMessage(arg0)); - this._createOrGetShellEnvDetection().applyEnvironmentDiff(env, arg1 === this._nonce); + this._createOrGetShellEnvDetection().setEnvironment(env, arg1 === this._nonce); } catch (e) { this._logService.warn('Failed to parse environment from shell integration sequence', arg0); } From f4e820c7b461ffeeaf537a9b48beda06c9942bf3 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 18:31:48 -0500 Subject: [PATCH 08/22] Revert "bring back TS side - TS side has no impact on perf" This reverts commit f6863cb9e53d01f326b604f4f30cee183f71f8a6. --- .../common/capabilities/capabilities.ts | 1 + .../shellEnvDetectionCapability.ts | 30 +++++++++++++++++-- .../common/xterm/shellIntegrationAddon.ts | 2 +- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/capabilities.ts b/src/vs/platform/terminal/common/capabilities/capabilities.ts index ba7597b9d926d..08e234517467e 100644 --- a/src/vs/platform/terminal/common/capabilities/capabilities.ts +++ b/src/vs/platform/terminal/common/capabilities/capabilities.ts @@ -158,6 +158,7 @@ export interface IShellEnvDetectionCapability { startEnvironmentSingleVar(isTrusted: boolean): void; setEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; endEnvironmentSingleVar(isTrusted: boolean): void; + applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean; } export const enum CommandInvalidationReason { diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 1a3e37ad8941c..06234fb71549d 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -58,8 +58,34 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (!this._pendingEnv) { return; } - this._env = this._pendingEnv; + const didDiffer = this.applyEnvironmentDiff(this._pendingEnv, isTrusted); + if (didDiffer) { + this._env = this._pendingEnv; + } this._pendingEnv = undefined; - this._onDidChangeEnv.fire(this._env); + } + // Returns true if the environment differs, and was updated. + applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean { + if (!isTrusted) { + return false; + } + + let envDiffers: boolean = false; + + for (const [key, value] of env.entries()) { + if (this._env.has(key) && this._env.get(key) === value) { + // Do nothing + } else if (value !== undefined) { + this._env.set(key, value); + envDiffers = true; + } + } + + if (envDiffers) { + this._onDidChangeEnv.fire(this._env); + return true; + } + + return false; } } diff --git a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts index 92b1851c275c8..feccd25c3b0b6 100644 --- a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts +++ b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts @@ -467,7 +467,7 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati if (arg0 !== undefined) { try { const env = JSON.parse(deserializeMessage(arg0)); - this._createOrGetShellEnvDetection().setEnvironment(env, arg1 === this._nonce); + this._createOrGetShellEnvDetection().applyEnvironmentDiff(env, arg1 === this._nonce); } catch (e) { this._logService.warn('Failed to parse environment from shell integration sequence', arg0); } From 39993b405969a1e274f55e3925f72e252e554115 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 18:34:49 -0500 Subject: [PATCH 09/22] better comments --- .../terminal/common/capabilities/shellEnvDetectionCapability.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 06234fb71549d..7bff1094f5375 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -65,6 +65,7 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv this._pendingEnv = undefined; } // Returns true if the environment differs, and was updated. + // This way we only fire an event if the environment actually changed. applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean { if (!isTrusted) { return false; From 73c6045d4500faef5f36becb80fd2d193c968be4 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 19:05:36 -0500 Subject: [PATCH 10/22] cleaner --- .../terminal/common/capabilities/capabilities.ts | 2 +- .../capabilities/shellEnvDetectionCapability.ts | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/capabilities.ts b/src/vs/platform/terminal/common/capabilities/capabilities.ts index 08e234517467e..50e5c91328979 100644 --- a/src/vs/platform/terminal/common/capabilities/capabilities.ts +++ b/src/vs/platform/terminal/common/capabilities/capabilities.ts @@ -158,7 +158,7 @@ export interface IShellEnvDetectionCapability { startEnvironmentSingleVar(isTrusted: boolean): void; setEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; endEnvironmentSingleVar(isTrusted: boolean): void; - applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean; + applyEnvironmentDiff(env: Map, isTrusted: boolean): void; } export const enum CommandInvalidationReason { diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 7bff1094f5375..e4269917175b7 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -58,17 +58,14 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (!this._pendingEnv) { return; } - const didDiffer = this.applyEnvironmentDiff(this._pendingEnv, isTrusted); - if (didDiffer) { - this._env = this._pendingEnv; - } + this.applyEnvironmentDiff(this._pendingEnv, isTrusted); this._pendingEnv = undefined; } // Returns true if the environment differs, and was updated. // This way we only fire an event if the environment actually changed. - applyEnvironmentDiff(env: Map, isTrusted: boolean): boolean { + applyEnvironmentDiff(env: Map, isTrusted: boolean): void { if (!isTrusted) { - return false; + return; } let envDiffers: boolean = false; @@ -84,9 +81,7 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (envDiffers) { this._onDidChangeEnv.fire(this._env); - return true; + return; } - - return false; } } From c6832d899086018f2454df648ee4050fc0153428 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 19:06:26 -0500 Subject: [PATCH 11/22] comment clean up --- .../terminal/common/capabilities/shellEnvDetectionCapability.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index e4269917175b7..fa12488850608 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -61,7 +61,7 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv this.applyEnvironmentDiff(this._pendingEnv, isTrusted); this._pendingEnv = undefined; } - // Returns true if the environment differs, and was updated. + // Check for environment differs, and was updated. // This way we only fire an event if the environment actually changed. applyEnvironmentDiff(env: Map, isTrusted: boolean): void { if (!isTrusted) { From 3386a5be952887257ce5bf36fc07ac5b42367156 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Wed, 22 Jan 2025 20:23:09 -0500 Subject: [PATCH 12/22] add way to track missing variable in bash side --- .../shellEnvDetectionCapability.ts | 3 ++ .../common/scripts/shellIntegration-bash.sh | 32 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index fa12488850608..1e6b85d976cbc 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -63,6 +63,9 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv } // Check for environment differs, and was updated. // This way we only fire an event if the environment actually changed. + + // TODO: Handle deleted variable on TS side. It is already one in bash side + // unsetEnvironmentSingleVar() applyEnvironmentDiff(env: Map, isTrusted: boolean): void { if (!isTrusted) { return; diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 4abf02fc39ebc..fb4c6a4dac341 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -239,6 +239,34 @@ updateEnvCache() { builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" } +trackMissingEnvVars() { + # Capture current environment variables in an array + local current_env_keys=() + while IFS='=' read -r key value; do + current_env_keys+=("$key") + done < <(env) + + # Compare vsc_env_keys with current_env_keys + for key in "${vsc_env_keys[@]}"; do + local found=false + for env_key in "${current_env_keys[@]}"; do + if [[ "$key" == "$env_key" ]]; then + found=true + break + fi + done + if [[ "$found" == false ]]; then + printf "Missing environment variable: %s\n" "$key" + unset 'vsc_env_keys[i]' + unset 'vsc_env_values[i]' + fi + done + + # Remove gaps from unset + vsc_env_keys=("${vsc_env_keys[@]}") + vsc_env_values=("${vsc_env_values[@]}") +} + __vsc_update_env() { start_time=$(date +%s) @@ -255,8 +283,10 @@ __vsc_update_env() { end_time=$(date +%s) elapsed_time=$(($end_time - $start_time)) - builtin printf "Time taken: $elapsed_time seconds\n" + # builtin printf "Time taken: $elapsed_time seconds\n" + trackMissingEnvVars + builtin printf "Time taken: $elapsed_time seconds\n" } __vsc_command_output_start() { From a00560ff23bb942ee57564671c13c2e8c5b2c204 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 23 Jan 2025 00:09:07 -0500 Subject: [PATCH 13/22] EnvSingleDelete --- .../common/capabilities/capabilities.ts | 1 + .../shellEnvDetectionCapability.ts | 16 ++++++++++++--- .../common/xterm/shellIntegrationAddon.ts | 20 +++++++++++++++++++ .../common/scripts/shellIntegration-bash.sh | 2 +- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/capabilities.ts b/src/vs/platform/terminal/common/capabilities/capabilities.ts index 50e5c91328979..c3924a37fb99b 100644 --- a/src/vs/platform/terminal/common/capabilities/capabilities.ts +++ b/src/vs/platform/terminal/common/capabilities/capabilities.ts @@ -157,6 +157,7 @@ export interface IShellEnvDetectionCapability { setEnvironment(envs: { [key: string]: string | undefined } | undefined, isTrusted: boolean): void; startEnvironmentSingleVar(isTrusted: boolean): void; setEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; + deleteEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void; endEnvironmentSingleVar(isTrusted: boolean): void; applyEnvironmentDiff(env: Map, isTrusted: boolean): void; } diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 1e6b85d976cbc..7b6b4de70e85a 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -61,11 +61,21 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv this.applyEnvironmentDiff(this._pendingEnv, isTrusted); this._pendingEnv = undefined; } + + deleteEnvironmentSingleVar(key: string, value: string | undefined, isTrusted: boolean): void { + if (!isTrusted) { + return; + } + if (key !== undefined && value !== undefined) { + this._env.delete(key); + this._pendingEnv?.delete(key); + this._onDidChangeEnv.fire(this._env); + } + return; + } + // Check for environment differs, and was updated. // This way we only fire an event if the environment actually changed. - - // TODO: Handle deleted variable on TS side. It is already one in bash side - // unsetEnvironmentSingleVar() applyEnvironmentDiff(env: Map, isTrusted: boolean): void { if (!isTrusted) { return; diff --git a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts index feccd25c3b0b6..693e937216c52 100644 --- a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts +++ b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts @@ -239,6 +239,16 @@ const enum VSCodeOscPt { */ EnvJson = 'EnvJson', + /** + * Delete a single environment variable from cached environment. + * + * Format: `OSC 633 ; EnvSingleDelete ; ; ; ` + * + * WARNING: This sequence is unfinalized, DO NOT use this in your shell integration script. + */ + + EnvSingleDelete = 'EnvSingleDelete', + /** * The start of the collecting user's environment variables individually. * Clears any environment residuals in previous sessions. @@ -478,6 +488,16 @@ export class ShellIntegrationAddon extends Disposable implements IShellIntegrati this._createOrGetShellEnvDetection().startEnvironmentSingleVar(args[0] === this._nonce); return true; } + case VSCodeOscPt.EnvSingleDelete: { + const arg0 = args[0]; + const arg1 = args[1]; + const arg2 = args[2]; + if (arg0 !== undefined && arg1 !== undefined) { + const env = deserializeMessage(arg1); + this._createOrGetShellEnvDetection().deleteEnvironmentSingleVar(arg0, env, arg2 === this._nonce); + } + return true; + } case VSCodeOscPt.EnvSingleEntry: { const arg0 = args[0]; const arg1 = args[1]; diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index fb4c6a4dac341..dba4e4d7e08c1 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -256,7 +256,7 @@ trackMissingEnvVars() { fi done if [[ "$found" == false ]]; then - printf "Missing environment variable: %s\n" "$key" + builtin printf '\e]633;EnvSingleDelete;%s;%s;%s\a' "${vsc_env_keys[i]}" "$(__vsc_escape_value "${vsc_env_values[i]}")" "$__vsc_nonce" unset 'vsc_env_keys[i]' unset 'vsc_env_values[i]' fi From 9c213c503ee6adb96f8e7816959b685c09e1b190 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Thu, 23 Jan 2025 00:45:08 -0500 Subject: [PATCH 14/22] better logic --- .../common/capabilities/shellEnvDetectionCapability.ts | 9 +++++---- .../terminal/common/scripts/shellIntegration-bash.sh | 6 ++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 7b6b4de70e85a..4b6a5b49b7518 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -69,13 +69,11 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (key !== undefined && value !== undefined) { this._env.delete(key); this._pendingEnv?.delete(key); - this._onDidChangeEnv.fire(this._env); } return; } - // Check for environment differs, and was updated. - // This way we only fire an event if the environment actually changed. + // Make sure to update this.env to the latest, fire event if there is a diff applyEnvironmentDiff(env: Map, isTrusted: boolean): void { if (!isTrusted) { return; @@ -86,7 +84,10 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv for (const [key, value] of env.entries()) { if (this._env.has(key) && this._env.get(key) === value) { // Do nothing - } else if (value !== undefined) { + } else if (this.env.has(key) && value !== this.env.get(key)) { + this._env.set(key, value); + envDiffers = true; + } else if (!this.env.has(key)) { this._env.set(key, value); envDiffers = true; } diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index dba4e4d7e08c1..0c00040dceffd 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -278,14 +278,12 @@ __vsc_update_env() { updateEnvCache "$key" "$value" done < <(env) + trackMissingEnvVars + builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce end_time=$(date +%s) elapsed_time=$(($end_time - $start_time)) - - # builtin printf "Time taken: $elapsed_time seconds\n" - - trackMissingEnvVars builtin printf "Time taken: $elapsed_time seconds\n" } From 7787b0e86424e5c6e85d1ad9f479d9152dfd4c84 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 23 Jan 2025 08:58:54 -0800 Subject: [PATCH 15/22] Todos --- .../common/scripts/shellIntegration-bash.sh | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 0c00040dceffd..ae3aaf76016fb 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -222,7 +222,9 @@ updateEnvCache() { local key="$1" local value="$2" + builtin printf "Start %s\n" $(date +%s%3N) for i in "${!vsc_env_keys[@]}"; do + # builtin printf "i $i $(date +%s%3N)\n" if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then if [[ "${vsc_env_values[$i]}" != "$value" ]]; then vsc_env_values[$i]="$value" @@ -242,6 +244,8 @@ updateEnvCache() { trackMissingEnvVars() { # Capture current environment variables in an array local current_env_keys=() + + # TODO: I think this will break for values that contain `=`, see `cut` command in `VSCODE_ENV_REPLACE` code while IFS='=' read -r key value; do current_env_keys+=("$key") done < <(env) @@ -251,12 +255,14 @@ trackMissingEnvVars() { local found=false for env_key in "${current_env_keys[@]}"; do if [[ "$key" == "$env_key" ]]; then + # TODO: Use 1 and 0 over true found=true break fi done if [[ "$found" == false ]]; then builtin printf '\e]633;EnvSingleDelete;%s;%s;%s\a' "${vsc_env_keys[i]}" "$(__vsc_escape_value "${vsc_env_values[i]}")" "$__vsc_nonce" + # TODO: Use `builtin unset` unset 'vsc_env_keys[i]' unset 'vsc_env_values[i]' fi @@ -269,22 +275,28 @@ trackMissingEnvVars() { __vsc_update_env() { - start_time=$(date +%s) + start_time=$(date +%s%3N) builtin printf "Start time: %s\n" "$start_time" + # TODO: Come up with shorter escape sequence ids (~10ms gain on Daniel's PC) + # TODO: Send everything the first time, don't bother diffing (array is empty) + # TODO: Only send EnvSingleStart, EnvSingleEnd when there is updated content builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce while IFS='=' read -r key value; do updateEnvCache "$key" "$value" done < <(env) + mid_time=$(date +%s%3N) trackMissingEnvVars builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce - end_time=$(date +%s) + end_time=$(date +%s%3N) elapsed_time=$(($end_time - $start_time)) - builtin printf "Time taken: $elapsed_time seconds\n" + builtin printf "updateEnvCache: $(($mid_time - $start_time)) ms\n" + builtin printf "trackMissingEnvVars: $(($end_time - $mid_time)) ms\n" + builtin printf "Total: $(($end_time - $start_time)) ms\n" } __vsc_command_output_start() { @@ -313,10 +325,6 @@ __vsc_command_complete() { builtin printf '\e]633;D;%s\a' "$__vsc_status" fi __vsc_update_cwd - - if [ "$__vsc_stable" = "0" ]; then - __vsc_update_env - fi } __vsc_update_prompt() { # in command execution @@ -346,7 +354,6 @@ __vsc_precmd() { fi __vsc_first_prompt=1 __vsc_update_prompt - if [ "$__vsc_stable" = "0" ]; then __vsc_update_env fi From 41df1b68abfa1a72804ac221f71e261ad1b67543 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 09:13:26 -0800 Subject: [PATCH 16/22] bring back env in command_complete DO NOT USE CUT as hinders perf --- .../common/scripts/shellIntegration-bash.sh | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index ae3aaf76016fb..9c9ea2ec7bc6f 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -222,7 +222,7 @@ updateEnvCache() { local key="$1" local value="$2" - builtin printf "Start %s\n" $(date +%s%3N) + # builtin printf "Start %s\n" $(date +%s%3N) for i in "${!vsc_env_keys[@]}"; do # builtin printf "i $i $(date +%s%3N)\n" if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then @@ -275,28 +275,34 @@ trackMissingEnvVars() { __vsc_update_env() { - start_time=$(date +%s%3N) - builtin printf "Start time: %s\n" "$start_time" - - # TODO: Come up with shorter escape sequence ids (~10ms gain on Daniel's PC) + # start_time=$(date +%s%3N) + # builtin printf "Start time: %s\n" "$start_time" # TODO: Send everything the first time, don't bother diffing (array is empty) - # TODO: Only send EnvSingleStart, EnvSingleEnd when there is updated content + builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce + # This surprisingly doesnt break even when value contains = or multiple = while IFS='=' read -r key value; do updateEnvCache "$key" "$value" done < <(env) - mid_time=$(date +%s%3N) + # IMPORTANT: Trying to use cut like this with echo really slow things down. + # while IFS= read -r line; do + # key=$(echo "$line" | cut -d '=' -f 1) + # value=$(echo "$line" | cut -d '=' -f 2-) + # updateEnvCache "$key" "$value" + # done < <(env) + + # mid_time=$(date +%s%3N) trackMissingEnvVars builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce - end_time=$(date +%s%3N) - elapsed_time=$(($end_time - $start_time)) - builtin printf "updateEnvCache: $(($mid_time - $start_time)) ms\n" - builtin printf "trackMissingEnvVars: $(($end_time - $mid_time)) ms\n" - builtin printf "Total: $(($end_time - $start_time)) ms\n" + # end_time=$(date +%s%3N) + # elapsed_time=$(($end_time - $start_time)) + # builtin printf "updateEnvCache: $(($mid_time - $start_time)) ms\n" + # builtin printf "trackMissingEnvVars: $(($end_time - $mid_time)) ms\n" + # builtin printf "Total: $(($end_time - $start_time)) ms\n" } __vsc_command_output_start() { @@ -325,6 +331,10 @@ __vsc_command_complete() { builtin printf '\e]633;D;%s\a' "$__vsc_status" fi __vsc_update_cwd + # IMPORTANT: We may have to bring back __vsc_update_env here, as now it takes one additional prompt execution for us to see unset env in effect + if [ "$__vsc_stable" = "0" ]; then + __vsc_update_env + fi } __vsc_update_prompt() { # in command execution From 8d8bf63a3c8ade871a2a65bf92ab4b62d0af1d67 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 11:47:21 -0800 Subject: [PATCH 17/22] do not diff for first time --- .../common/scripts/shellIntegration-bash.sh | 34 ++++++------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 9c9ea2ec7bc6f..0252a75f1640c 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -274,35 +274,23 @@ trackMissingEnvVars() { } __vsc_update_env() { - - # start_time=$(date +%s%3N) - # builtin printf "Start time: %s\n" "$start_time" - # TODO: Send everything the first time, don't bother diffing (array is empty) - builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce - # This surprisingly doesnt break even when value contains = or multiple = - while IFS='=' read -r key value; do - updateEnvCache "$key" "$value" - done < <(env) - - # IMPORTANT: Trying to use cut like this with echo really slow things down. - # while IFS= read -r line; do - # key=$(echo "$line" | cut -d '=' -f 1) - # value=$(echo "$line" | cut -d '=' -f 2-) - # updateEnvCache "$key" "$value" - # done < <(env) + if [[ -z ${vsc_env_keys[@]} ]] && [[ -z ${vsc_env_values[@]} ]]; then + while IFS='=' read -r key value; do + vsc_env_keys+=("$key") + vsc_env_values+=("$value") + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + done < <(env) + else + while IFS='=' read -r key value; do + updateEnvCache "$key" "$value" + done < <(env) + fi - # mid_time=$(date +%s%3N) trackMissingEnvVars builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce - - # end_time=$(date +%s%3N) - # elapsed_time=$(($end_time - $start_time)) - # builtin printf "updateEnvCache: $(($mid_time - $start_time)) ms\n" - # builtin printf "trackMissingEnvVars: $(($end_time - $mid_time)) ms\n" - # builtin printf "Total: $(($end_time - $start_time)) ms\n" } __vsc_command_output_start() { From 224bfcf5c455865ab0ac78ee2644ddcd724b8240 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 11:56:21 -0800 Subject: [PATCH 18/22] use 1,0 over true,false AND use builtin unset --- .../terminal/common/scripts/shellIntegration-bash.sh | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 0252a75f1640c..5030f36283e5d 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -252,19 +252,17 @@ trackMissingEnvVars() { # Compare vsc_env_keys with current_env_keys for key in "${vsc_env_keys[@]}"; do - local found=false + local found=0 for env_key in "${current_env_keys[@]}"; do if [[ "$key" == "$env_key" ]]; then - # TODO: Use 1 and 0 over true - found=true + found=1 break fi done - if [[ "$found" == false ]]; then + if [ "$found" = 0 ]; then builtin printf '\e]633;EnvSingleDelete;%s;%s;%s\a' "${vsc_env_keys[i]}" "$(__vsc_escape_value "${vsc_env_values[i]}")" "$__vsc_nonce" - # TODO: Use `builtin unset` - unset 'vsc_env_keys[i]' - unset 'vsc_env_values[i]' + builtin unset 'vsc_env_keys[i]' + builtin unset 'vsc_env_values[i]' fi done From 84eb38c126889077abce64003e78e5aa91ef225b Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 18:20:34 -0800 Subject: [PATCH 19/22] use associative array depending on bash > 4.0 --- .../shellEnvDetectionCapability.ts | 1 + .../common/scripts/shellIntegration-bash.sh | 83 +++++++++++++++---- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts index 4b6a5b49b7518..0dcbed0567533 100644 --- a/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts +++ b/src/vs/platform/terminal/common/capabilities/shellEnvDetectionCapability.ts @@ -69,6 +69,7 @@ export class ShellEnvDetectionCapability extends Disposable implements IShellEnv if (key !== undefined && value !== undefined) { this._env.delete(key); this._pendingEnv?.delete(key); + this._onDidChangeEnv.fire(this._env); } return; } diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 5030f36283e5d..69c5e9e40b698 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -12,6 +12,14 @@ VSCODE_SHELL_INTEGRATION=1 vsc_env_keys=() vsc_env_values=() +use_associative_array=0 +bash_major_version=$(echo $BASH_VERSION | cut -d'.' -f1) + +if [ "$bash_major_version" -gt 4 ]; then + use_associative_array=1 + # Associative arrays are only available in bash 4.0+ + declare -A vsc_aa_env +fi # Run relevant rc/profile only if shell integration has been injected, not when run manually if [ "$VSCODE_INJECTION" == "1" ]; then @@ -222,13 +230,10 @@ updateEnvCache() { local key="$1" local value="$2" - # builtin printf "Start %s\n" $(date +%s%3N) for i in "${!vsc_env_keys[@]}"; do - # builtin printf "i $i $(date +%s%3N)\n" if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then if [[ "${vsc_env_values[$i]}" != "$value" ]]; then vsc_env_values[$i]="$value" - # printf "Updated value for %s\n" "$key" builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" fi return @@ -237,15 +242,44 @@ updateEnvCache() { vsc_env_keys+=("$key") vsc_env_values+=("$value") - # printf "Added new variable %s\n" "$key" builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" } +updateEnvCacheAA() { + local key="$1" + local value="$2" + + if [[ "${vsc_aa_env[$key]}" != "$value" ]]; then + vsc_aa_env["$key"]="$value" + builtin printf 'I found smth to update!!! \n ;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + fi +} + +trackMissingEnvVarsAA() { + if [ "$use_associative_array" = 1 ]; then + declare -A currentEnvMap + while IFS='=' read -r key value; do + currentEnvMap["$key"]="$value" + done < <(env) + + for key in "${!vsc_aa_env[@]}"; do + if [ -z "${currentEnvMap[$key]}" ]; then + builtin printf 'Im pretty sure we are deleting something here \n' + # print which key and value we are deleting + builtin printf '%s;%s;%s\a' "$key" "$(__vsc_escape_value "${vsc_aa_env[$key]}")" "$__vsc_nonce" + builtin printf 'Did you see that? \n' + builtin printf '\e]633;EnvSingleDelete;%s;%s;%s\a' "$key" "$(__vsc_escape_value "${vsc_aa_env[$key]}")" "$__vsc_nonce" + builtin unset "vsc_aa_env[$key]" + fi + done + fi +} + trackMissingEnvVars() { # Capture current environment variables in an array local current_env_keys=() - # TODO: I think this will break for values that contain `=`, see `cut` command in `VSCODE_ENV_REPLACE` code while IFS='=' read -r key value; do current_env_keys+=("$key") done < <(env) @@ -274,20 +308,37 @@ trackMissingEnvVars() { __vsc_update_env() { builtin printf '\e]633;EnvSingleStart;%s;\a' $__vsc_nonce - if [[ -z ${vsc_env_keys[@]} ]] && [[ -z ${vsc_env_values[@]} ]]; then - while IFS='=' read -r key value; do - vsc_env_keys+=("$key") - vsc_env_values+=("$value") - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" - done < <(env) + if [ "$use_associative_array" = 1 ]; then + if [ ${#vsc_aa_env[@]} -eq 0 ]; then + # Associative array is empty, do not diff, just add + while IFS='=' read -r key value; do + vsc_aa_env["$key"]="$value" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + done < <(env) + else + # Diff + while IFS='=' read -r key value; do + updateEnvCacheAA "$key" "$value" + done < <(env) + trackMissingEnvVarsAA + fi + else - while IFS='=' read -r key value; do - updateEnvCache "$key" "$value" - done < <(env) - fi + if [[ -z ${vsc_env_keys[@]} ]] && [[ -z ${vsc_env_values[@]} ]]; then + while IFS='=' read -r key value; do + vsc_env_keys+=("$key") + vsc_env_values+=("$value") + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + done < <(env) + else + while IFS='=' read -r key value; do + updateEnvCache "$key" "$value" + done < <(env) + trackMissingEnvVars + fi - trackMissingEnvVars + fi builtin printf '\e]633;EnvSingleEnd;%s;\a' $__vsc_nonce } From 6ae101135ee0fb784507cda5d2dc5f8fba3ce789 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 19:27:55 -0800 Subject: [PATCH 20/22] clean up - associative approach completed --- .../common/scripts/shellIntegration-bash.sh | 60 +++++++++---------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 69c5e9e40b698..3d24328b768a5 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -225,34 +225,14 @@ __vsc_update_cwd() { builtin printf '\e]633;P;Cwd=%s\a' "$(__vsc_escape_value "$__vsc_cwd")" } - -updateEnvCache() { - local key="$1" - local value="$2" - - for i in "${!vsc_env_keys[@]}"; do - if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then - if [[ "${vsc_env_values[$i]}" != "$value" ]]; then - vsc_env_values[$i]="$value" - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" - fi - return - fi - done - - vsc_env_keys+=("$key") - vsc_env_values+=("$value") - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" -} - updateEnvCacheAA() { local key="$1" local value="$2" - - if [[ "${vsc_aa_env[$key]}" != "$value" ]]; then - vsc_aa_env["$key"]="$value" - builtin printf 'I found smth to update!!! \n ;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" - builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + if [ "$use_associative_array" = 1 ]; then + if [[ "${vsc_aa_env[$key]}" != "$value" ]]; then + vsc_aa_env["$key"]="$value" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + fi fi } @@ -265,10 +245,6 @@ trackMissingEnvVarsAA() { for key in "${!vsc_aa_env[@]}"; do if [ -z "${currentEnvMap[$key]}" ]; then - builtin printf 'Im pretty sure we are deleting something here \n' - # print which key and value we are deleting - builtin printf '%s;%s;%s\a' "$key" "$(__vsc_escape_value "${vsc_aa_env[$key]}")" "$__vsc_nonce" - builtin printf 'Did you see that? \n' builtin printf '\e]633;EnvSingleDelete;%s;%s;%s\a' "$key" "$(__vsc_escape_value "${vsc_aa_env[$key]}")" "$__vsc_nonce" builtin unset "vsc_aa_env[$key]" fi @@ -276,15 +252,33 @@ trackMissingEnvVarsAA() { fi } +updateEnvCache() { + local key="$1" + local value="$2" + + for i in "${!vsc_env_keys[@]}"; do + if [[ "${vsc_env_keys[$i]}" == "$key" ]]; then + if [[ "${vsc_env_values[$i]}" != "$value" ]]; then + vsc_env_values[$i]="$value" + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" + fi + return + fi + done + + vsc_env_keys+=("$key") + vsc_env_values+=("$value") + builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" +} + trackMissingEnvVars() { - # Capture current environment variables in an array local current_env_keys=() while IFS='=' read -r key value; do current_env_keys+=("$key") done < <(env) - # Compare vsc_env_keys with current_env_keys + # Compare vsc_env_keys with user's current_env_keys for key in "${vsc_env_keys[@]}"; do local found=0 for env_key in "${current_env_keys[@]}"; do @@ -316,7 +310,7 @@ __vsc_update_env() { builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" done < <(env) else - # Diff + # Diff approach for associative array while IFS='=' read -r key value; do updateEnvCacheAA "$key" "$value" done < <(env) @@ -325,12 +319,14 @@ __vsc_update_env() { else if [[ -z ${vsc_env_keys[@]} ]] && [[ -z ${vsc_env_values[@]} ]]; then + # Non associative arrays are both empty, do not diff, just add while IFS='=' read -r key value; do vsc_env_keys+=("$key") vsc_env_values+=("$value") builtin printf '\e]633;EnvSingleEntry;%s;%s;%s\a' "$key" "$(__vsc_escape_value "$value")" "$__vsc_nonce" done < <(env) else + # Diff approach for non-associative arrays while IFS='=' read -r key value; do updateEnvCache "$key" "$value" done < <(env) From ad4367f8c8a94a0eec21ad2b1c9f9ea1f0657bf9 Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 19:54:32 -0800 Subject: [PATCH 21/22] get ready for review --- .../common/extHostTerminalShellIntegration.ts | 48 +++++++++---------- .../common/scripts/shellIntegration-bash.sh | 2 +- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts index 5cbb8edcb6eb6..105dca877a05c 100644 --- a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts +++ b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts @@ -55,30 +55,30 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH })); // Convenient test code: - this.onDidChangeTerminalShellIntegration(e => { - console.log('*** onDidChangeTerminalShellIntegration', e.shellIntegration.env); - }); - this.onDidStartTerminalShellExecution(async e => { - console.log('*** onDidStartTerminalShellExecution', e); - // new Promise(r => { - // (async () => { - // for await (const d of e.execution.read()) { - // console.log('data2', d); - // } - // })(); - // }); - for await (const d of e.execution.read()) { - console.log('data', d); - } - }); - this.onDidEndTerminalShellExecution(e => { - console.log('*** onDidEndTerminalShellExecution', e); - }); - setTimeout(() => { - console.log('before executeCommand(\"echo hello\")'); - Array.from(this._activeShellIntegrations.values())[0].value.executeCommand('echo hello'); - console.log('after executeCommand(\"echo hello\")'); - }, 4000); + // this.onDidChangeTerminalShellIntegration(e => { + // console.log('*** onDidChangeTerminalShellIntegration', e.shellIntegration.env); + // }); + // this.onDidStartTerminalShellExecution(async e => { + // console.log('*** onDidStartTerminalShellExecution', e); + // // new Promise(r => { + // // (async () => { + // // for await (const d of e.execution.read()) { + // // console.log('data2', d); + // // } + // // })(); + // // }); + // for await (const d of e.execution.read()) { + // console.log('data', d); + // } + // }); + // this.onDidEndTerminalShellExecution(e => { + // console.log('*** onDidEndTerminalShellExecution', e); + // }); + // setTimeout(() => { + // console.log('before executeCommand(\"echo hello\")'); + // Array.from(this._activeShellIntegrations.values())[0].value.executeCommand('echo hello'); + // console.log('after executeCommand(\"echo hello\")'); + // }, 4000); } public $shellIntegrationChange(instanceId: number): void { diff --git a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh index 3d24328b768a5..b2382744ae893 100644 --- a/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh +++ b/src/vs/workbench/contrib/terminal/common/scripts/shellIntegration-bash.sh @@ -364,7 +364,7 @@ __vsc_command_complete() { builtin printf '\e]633;D;%s\a' "$__vsc_status" fi __vsc_update_cwd - # IMPORTANT: We may have to bring back __vsc_update_env here, as now it takes one additional prompt execution for us to see unset env in effect + if [ "$__vsc_stable" = "0" ]; then __vsc_update_env fi From 76edbf37683bee3d79387222453e602864a15a3d Mon Sep 17 00:00:00 2001 From: Anthony Kim Date: Fri, 24 Jan 2025 19:58:57 -0800 Subject: [PATCH 22/22] dont leave a mess --- src/vs/workbench/api/common/extHostTerminalShellIntegration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts index 105dca877a05c..076f1b725cc36 100644 --- a/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts +++ b/src/vs/workbench/api/common/extHostTerminalShellIntegration.ts @@ -56,7 +56,7 @@ export class ExtHostTerminalShellIntegration extends Disposable implements IExtH // Convenient test code: // this.onDidChangeTerminalShellIntegration(e => { - // console.log('*** onDidChangeTerminalShellIntegration', e.shellIntegration.env); + // console.log('*** onDidChangeTerminalShellIntegration', e); // }); // this.onDidStartTerminalShellExecution(async e => { // console.log('*** onDidStartTerminalShellExecution', e);