Skip to content

Commit a3c5ee6

Browse files
Replace bespoke pwsh check with an integration test and trim comments
The vi edit-mode invariant now rides on the integration harness next to the zsh regression test for the same defect (GH #7099), which exercises a real Warpified session instead of a function lifted out of the bootstrap by AST surgery. Drops script/test_pwsh_bootstrap and its CI and presubmit wiring. Also cuts the rationale in pwsh.ps1 down to the part the code cannot say for itself; the measurements and the prior-art history live on the PR and the issue.
1 parent 727b730 commit a3c5ee6

7 files changed

Lines changed: 51 additions & 151 deletions

File tree

.github/workflows/ci.yml

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -681,10 +681,6 @@ jobs:
681681
run: ./script/lint_powershell -ci
682682
shell: pwsh
683683

684-
- name: Run pwsh bootstrap checks
685-
run: ./script/test_pwsh_bootstrap -ci
686-
shell: pwsh
687-
688684
- name: Validate repo-sync markers
689685
uses: warpdotdev/repo-sync/actions/validate-markers@main
690686

app/assets/bundled/bootstrap/pwsh.ps1

Lines changed: 12 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,6 @@ $null = New-Module -Name Warp-Module -ScriptBlock {
235235
rcfiles_start_time = "$rcStartTime"
236236
rcfiles_end_time = "$rcEndTime"
237237
shell_plugins = ''
238-
# The user's preference, not the session's live edit mode, which Warp overrides.
239238
vi_mode_enabled = $(if ($script:viEditModeOverridden) { '1' } else { '' })
240239
os_category = $osCategory
241240
linux_distribution = "$linuxDistribution"
@@ -352,41 +351,19 @@ $null = New-Module -Name Warp-Module -ScriptBlock {
352351
# it is set to $false.
353352
$script:commandNotFound = $false
354353

355-
# Set when Warp turns off PSReadLine's vi edit mode for this session, so that the
356-
# 'Bootstrapped' payload can still report the user's preference for vi keybindings.
357-
# See Warp-Configure-PSReadLine for why the edit mode is overridden.
354+
# Reports the user's vi preference in the 'Bootstrapped' payload even though the session's
355+
# live edit mode is overridden. See Warp-Configure-PSReadLine.
358356
$script:viEditModeOverridden = $false
359357

360358
function Warp-Configure-PSReadLine {
361-
# Warp drives PSReadLine over the PTY with two chords that are a bare Escape plus a
362-
# printable character: 'Alt+1' reports and drains the input buffer on every prompt, and
363-
# 'Alt+2' clears it ahead of every submitted command. ConPTY readily delivers the two
364-
# bytes in separate reads, and a chord split that way never dispatches, because 'Alt+1'
365-
# is its own dispatch entry rather than the two-key chord 'Escape,1'. What the stray
366-
# Escape then costs is entirely down to the edit mode. Measured on PSReadLine 2.3.5:
367-
#
368-
# Emacs Escape is bound to no function at all, only ever the first key of two-key
369-
# chords ('Escape,f' and friends), so the fragment resolves to an unbound chord
370-
# and is swallowed, leaving the buffer untouched.
371-
# Windows Escape is RevertLine, so the fragment clears the line and the digit then
372-
# self-inserts. That is the stray '1'/'2' prefix of GH #10891.
373-
# Vi Escape is ViCommandMode, so the fragment strands the editor in command mode
374-
# and everything Warp writes next is reinterpreted as vi edits. A submitted
375-
# 'echo hello' arrives as 'o hello'.
376-
#
377-
# Only vi is overridden, because only vi reinterprets the command. Windows edit mode is
378-
# PSReadLine's default on Windows, so forcing Emacs there would reset the key handlers of
379-
# every Windows user to address a milder, separately tracked defect. Nothing the user can
380-
# see changes: their keystrokes go through Warp's own input editor, never PSReadLine's. A
381-
# vi user does lose PSReadLine handlers registered elsewhere, since changing the edit mode
382-
# resets them all; that is an accepted trade, because those handlers are equally
383-
# unreachable inside Warp. fish resolves the same conflict the same way, resetting
384-
# 'fish_key_bindings' to the defaults on every precmd.
385-
#
386-
# Reasserting this on every prompt, rather than once after the user's profile is sourced,
387-
# also covers an edit mode set later in the session, from the next prompt onward. The
388-
# check keeps that cheap and non-destructive, and the handler reset it guards against is
389-
# why this has to precede the Set-PSReadLineKeyHandler calls below.
359+
# The 'Alt+1' and 'Alt+2' chords below reach PSReadLine as a bare Escape plus a digit, and
360+
# a chord split across reads -- which ConPTY readily produces -- never dispatches. Emacs
361+
# swallows the stray Escape, but Vi binds it to ViCommandMode, so everything Warp writes
362+
# next is reinterpreted as vi edits and 'echo hello' submits as 'o hello'. Only vi is
363+
# overridden, because only vi corrupts the command; the milder stray-digit damage under
364+
# the 'Windows' edit mode is GH #10891. The override costs a vi user nothing they can see,
365+
# since a Warpified session's keystrokes go through Warp's input editor rather than
366+
# PSReadLine's, and it must precede the bindings because it resets every key handler.
390367
if ((Get-PSReadLineOption).EditMode -eq 'Vi') {
391368
$script:viEditModeOverridden = $true
392369
Set-PSReadLineOption -EditMode Emacs
@@ -997,10 +974,8 @@ $null = New-Module -Name Warp-Module -ScriptBlock {
997974

998975
function Warp-Finish-Bootstrap {
999976
param([decimal]$rcStartTime, [decimal]$rcEndTime)
1000-
# Sourcing the user's profiles can reconfigure PSReadLine out from under the bootstrap's
1001-
# own Warp-Precmd call, which ran before them: 'Set-PSReadLineOption -EditMode Vi' resets
1002-
# every key handler and so unbinds Warp's chords. Reconfiguring here, rather than waiting
1003-
# for the first prompt, also means the payload Warp-Bootstrapped sends below reports it.
977+
# The profiles just sourced ran after the bootstrap's own Warp-Precmd, and setting the vi
978+
# edit mode there unbinds Warp's chords by resetting every key handler.
1004979
Warp-Configure-PSReadLine
1005980

1006981
# This is the closest we can get in PowerShell to a proper preexec hook. We wrap the

crates/integration/src/bin/integration.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,7 @@ fn register_tests() -> HashMap<&'static str, BoxedBuilderFn> {
259259
register_test!(test_bash_bootstraps_with_prompt_command_array_that_sets_ps1);
260260
register_test!(test_zsh_bootstraps_with_nounset_option);
261261
register_test!(test_zsh_cursor_mode_vi_bindings_do_not_corrupt_commands);
262+
register_test!(test_pwsh_vi_edit_mode_does_not_corrupt_commands);
262263
register_test!(test_ssh_wrapper_into_bash);
263264
register_test!(test_ssh_wrapper_into_zsh);
264265
register_test!(test_ssh_into_fish);

crates/integration/src/test/bootstrapping.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,42 @@ zle -N zle-line-init
364364
))
365365
}
366366

367+
/// Regression test for CORE-3804: a profile that selects PSReadLine's vi edit mode must not
368+
/// corrupt submitted commands. Warp's input-reporting and kill-buffer chords reach PSReadLine as
369+
/// a bare Escape plus a digit, which vi binds to ViCommandMode and DigitArgument, so before the
370+
/// fix a submitted `echo hello` arrived as `o hello`.
371+
pub fn test_pwsh_vi_edit_mode_does_not_corrupt_commands() -> Builder {
372+
new_builder()
373+
.set_should_run_test(|| {
374+
let (starter, _) = current_shell_starter_and_version();
375+
matches!(starter.shell_type(), shell::ShellType::PowerShell)
376+
})
377+
.with_setup(|utils| {
378+
let dir = utils.test_dir();
379+
write_rc_files_for_test(
380+
dir,
381+
"Set-PSReadLineOption -EditMode Vi",
382+
[ShellRcType::PowerShell],
383+
);
384+
})
385+
.with_step(wait_until_bootstrapped_single_pane_for_tab(0))
386+
.with_step(clear_blocklist_to_remove_bootstrapped_blocks())
387+
.with_step(execute_command_for_single_terminal_in_tab(
388+
0,
389+
"echo vi_edit_mode_ok".to_string(),
390+
ExpectedExitStatus::Success,
391+
"vi_edit_mode_ok",
392+
))
393+
// The leading token is what vi command mode eats first, so a second command guards the
394+
// failure mode where only part of the text survives.
395+
.with_step(execute_command_for_single_terminal_in_tab(
396+
0,
397+
"Write-Output second_command_ok".to_string(),
398+
ExpectedExitStatus::Success,
399+
"second_command_ok",
400+
))
401+
}
402+
367403
pub fn test_bash_bootstraps_with_prompt_command_array_that_sets_ps1() -> Builder {
368404
new_builder()
369405
.set_should_run_test(|| {

crates/integration/tests/integration/shell_integration_tests.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ integration_tests! {
7878
// Tests zsh-specific behavior with nounset option.
7979
test_zsh_bootstraps_with_nounset_option,
8080
test_zsh_cursor_mode_vi_bindings_do_not_corrupt_commands,
81+
// Tests PowerShell-specific behavior with PSReadLine's vi edit mode.
82+
test_pwsh_vi_edit_mode_does_not_corrupt_commands,
8183

8284
// Tests of ssh wrapper logic from bootstrap script.
8385
test_ssh_wrapper_into_bash,

script/presubmit

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ if command -v pwsh /dev/null 2>&1; then
5353
echo "Running PSScriptAnalyzer..."
5454
./script/lint_powershell -ci
5555
echo "PsScriptAnalyzer succeeded..."
56-
echo "Running pwsh bootstrap checks..."
57-
./script/test_pwsh_bootstrap -ci
58-
echo "pwsh bootstrap checks succeeded..."
5956
elif [ "${GITHUB_ACTIONS}" == "true" ]; then
6057
# If we are in CI, and fail to find powershell, we should automatically fail
6158
echo "No powershell installation detected! Aborting!"

script/test_pwsh_bootstrap

Lines changed: 0 additions & 107 deletions
This file was deleted.

0 commit comments

Comments
 (0)