8000 Merge #19438 from 3N4N/fix/pwsh · neovim/neovim@01721aa · GitHub 8000
[go: up one dir, main page]
Skip to content

Commit 01721aa

Browse files
justinmkjustinmk
authored
Merge #19438 from 3N4N/fix/pwsh
Reverts #16271 Fixs #15913 Problem: Since #16271, `make_filter_cmd` uses `Start-Process` cmdlet to execute the user provided shell command for `:%!`. `Start-Process` requires the command to be split into the shell command and its arguments. This was implemented in #19268 by parsing (splitting the user-provided command at the first space) which didn't handle cases such as -- - commands with escaped space in their filepath - quoted commands with space in their filepath Solution: Use piping. The total shell command formats (excluding noise of unimportant parameters): 1. Before #16271 ```powershell pwsh -C "(shell_cmd) < tmp.in | 2>&1 Out-File -Encoding UTF8 <tmp.out>" # not how powershell commands work ``` 2. Since #16271 ```powershell pwsh -C "Start-Process shell_cmd -RedirectStandardInput <tmp.in> -RedirectStandardOutput <tmp.out>" # doesn't handle executable path with space in it # doesn't write error to <tmp.out> ``` 3. This PR ```powershell pwsh -C "& { Get-Content <tmp.in> | & 'path\with space\to\shell_cmd.exe' arg1 arg2 } 2>&1 | Out-File -Encoding UTF8 <tmp.out>" # also works with forward slash in the filepath # also works with double quotes around shell command ``` After this PR, the user can use the following formats: :%!c:\Program` Files\Git\usr\bin\sort.exe :%!'c:\Program Files\Git\usr\bin\sort.exe' :%!"c:\Program Files\Git\usr\bin\sort.exe" :%!"c:\Program` Files\Git\usr\bin\sort.exe" They can even chain different commands: :%!"c:\Program` Files\Git\usr\bin\sort.exe" | sort.exe -r But if they want to call a stringed executable path, they have to provide the Invoke-Command operator (&). In fact, the first stringed executable path also needs this & operator, but this PR adds that behind the scene. :%!"c:\Program` Files\Git\usr\bin\sort.exe" | sort.exe -r | & 'c:\Program Files\Git\usr\bin\sort.exe' ## What this PR solves - Having to parse the user-provided bang ex-command (for splitting into shell cmd and its args). - Removes a lot of human-unreadable `#ifdef` blocks. - Accepting escaped spaces in executable path. - Accepting quoted string of executable path. - Redirects error and exception to tmp.out (exception for when `wrong_cmd.exe not found`) ## What this PR doesn't solve - Handling wrongly escaped path to executable, which the user may pass because of cmdline tab-completion. #18592 ## Edge cases - (Not handled) If the user themself provides the `&` sign (means `call this.exe` in powershell) - (Not handled) Use `-Encoding utf8` parameter for `Get-Content`? - (Handled) Doesn't write to tmp.out if shell command is not found. - fix: use anonymous function (`{wrong_cmd.exe}`). ## Changes other than `make_filter_cmd()` function - Encoding for piping to external executables. See BOM-less UTF8: PowerShell/PowerShell#4681
2 parents 618d21f + f2482b3 commit 01721aa

File tree

4 files changed

+47
-59
lines changed

4 files changed

+47
-59
lines changed

‎runtime/doc/options.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5357,7 +5357,7 @@ A jump table for the options with a short description can be found at |Q_op|.
53575357
To use PowerShell: >
53585358
let &shell = executable('pwsh') ? 'pwsh' : 'powershell'
53595359
let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command [Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;'
5360-
let &shellredir = '-RedirectStandardOutput %s -NoNewWindow -Wait'
5360+
let &shellredir = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
53615361
let &shellpipe = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
53625362
set shellquote= shellxquote=
53635363

‎src/nvim/ex_cmds.c

Lines changed: 41 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,84 +1544,72 @@ char *make_filter_cmd(char *cmd, char *itmp, char *otmp)
15441544

15451545
size_t len = strlen(cmd) + 1; // At least enough space for cmd + NULL.
15461546

1547-
len += is_fish_shell ? sizeof("begin; " "; end") - 1
1548-
: is_pwsh ? sizeof("Start-Process ")
1549-
: sizeof("(" ")") - 1;
1547+
len += is_fish_shell ? sizeof("begin; " "; end") - 1
1548+
: !is_pwsh ? sizeof("(" ")") - 1
1549+
: 0;
15501550

15511551
if (itmp != NULL) {
1552-
len += is_pwsh ? strlen(itmp) + sizeof(" -RedirectStandardInput ")
1552+
len += is_pwsh ? strlen(itmp) + sizeof("& { Get-Content " " | & " " }") - 1
15531553
: strlen(itmp) + sizeof(" { " " < " " } ") - 1;
15541554
}
15551555
if (otmp != NULL) {
15561556
len += strlen(otmp) + strlen(p_srr) + 2; // two extra spaces (" "),
15571557
}
15581558

1559-
const char *const cmd_args = strchr(cmd, ' ');
1560-
len += (is_pwsh && cmd_args)
1561-
? strlen(" -ArgumentList ") + 2 // two extra quotes
1562-
: 0;
1563-
15641559
char *const buf = xmalloc(len);
15651560

15661561
if (is_pwsh) {
1567-
xstrlcpy(buf, "Start-Process ", len);
1568-
if (cmd_args == NULL) {
1569-
xstrlcat(buf, cmd, len);
1562+
if (itmp != NULL) {
1563+
xstrlcpy(buf, "& { Get-Content ", len - 1); // FIXME: should we add "-Encoding utf8"?
1564+
xstrlcat(buf, (const char *)itmp, len - 1);
1565+
xstrlcat(buf, " | & ", len - 1); // FIXME: add `&` ourself or leave to user?
1566+
xstrlcat(buf, cmd, len - 1);
1567+
xstrlcat(buf, " }", len - 1);
15701568
} else {
1571-
xstrlcpy(buf + strlen(buf), cmd, (size_t)(cmd_args - cmd + 1));
1572-
xstrlcat(buf, " -ArgumentList \"", len);
1573-
xstrlcat(buf, cmd_args + 1, len); // +1 to skip the leading space.
1574-
xstrlcat(buf, "\"", len);
1569+
xstrlcpy(buf, cmd, len - 1);
15751570
}
1571+
} else {
15761572
#if defined(UNIX)
15771573
// Put delimiters around the command (for concatenated commands) when
15781574
// redirecting input and/or output.
1579-
} else if (itmp != NULL || otmp != NULL) {
1580-
char *fmt = is_fish_shell ? "begin; %s; end"
1581-
: "(%s)";
1582-
vim_snprintf(buf, len, fmt, cmd);
1583-
#endif
1584-
// For shells that don't understand braces around commands, at least allow
1585-
// the use of commands in a pipe.
1586-
} else {
1587-
xstrlcpy(buf, cmd, len);
1588-
}
1589-
1590-
#if defined(UNIX)
1591-
if (itmp != NULL) {
1592-
if (is_pwsh) {
1593-
xstrlcat(buf, " -RedirectStandardInput ", len - 1);
1575+
if (itmp != NULL || otmp != NULL) {
1576+
char *fmt = is_fish_shell ? "begin; %s; end"
1577+
: "(%s)";
1578+
vim_snprintf(buf, len, fmt, cmd);
15941579
} else {
1580+
xstrlcpy(buf, cmd, len);
1581+
}
1582+
1583+
if (itmp != NULL) {
15951584
xstrlcat(buf, " < ", len - 1);
1585+
xstrlcat(buf, (const char *)itmp, len - 1);
15961586
}
1597-
xstrlcat(buf, itmp, len - 1);
1598-
}
15991587
#else
1600-
if (itmp != NULL) {
1601-
// If there is a pipe, we have to put the '<' in front of it.
1602-
// Don't do this when 'shellquote' is not empty, otherwise the
1603-
// redirection would be inside the quotes.
1604-
if (*p_shq == NUL) {
1605-
char *const p = find_pipe(buf);
1606-
if (p != NULL) {
1607-
*p = NUL;
1588+
// For shells that don't understand braces around commands, at least allow
1589+
// the use of commands in a pipe.
1590+
xstrlcpy(buf, (char *)cmd, len);
1591+
if (itmp != NULL) {
1592+
// If there is a pipe, we have to put the '<' in front of it.
1593+
// Don't do this when 'shellquote' is not empty, otherwise the
1594+
// redirection would be inside the quotes.
1595+
if (*p_shq == NUL) {
1596+
char *const p = find_pipe(buf);
1597+
if (p != NULL) {
1598+
*p = NUL;
1599+
}
16081600
}
1609-
}
1610-
if (is_pwsh) {
1611-
xstrlcat(buf, " -RedirectStandardInput ", len);
1612-
} else {
16131601
xstrlcat(buf, " < ", len);
1614-
}
1615-
xstrlcat(buf, itmp, len);
1616-
if (*p_shq == NUL) {
1617-
const char *const p = find_pipe(cmd);
1618-
if (p != NULL) {
1619-
xstrlcat(buf, " ", len - 1); // Insert a space before the '|' for DOS
1620-
xstrlcat(buf, p, len - 1);
1602+
xstrlcat(buf, (const char *)itmp, len);
1603+
if (*p_shq == NUL) {
1604+
const char *const p = find_pipe((const char *)cmd);
1605+
if (p != NULL) {
1606+
xstrlcat(buf, " ", len - 1); // Insert a space before the '|' for DOS
1607+
xstrlcat(buf, p, len - 1);
1608+
}
16211609
}
16221610
}
1623-
}
16241611
#endif
1612+
}
16251613
if (otmp != NULL) {
16261614
append_redir(buf, len, p_srr, otmp);
16271615
}

‎test/functional/helpers.lua

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -562,16 +562,16 @@ function module.set_shell_powershell(fake)
562562
assert(found)
563563
end
564564
local shell = found and (iswin() and 'powershell' or 'pwsh') or module.testprg('pwsh-test')
565-
local set_encoding = '[Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;'
565+
local set_encoding = '[Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.UTF8Encoding]::new();'
566566
local cmd = set_encoding..'Remove-Item -Force '..table.concat(iswin()
567-
and {'alias:cat', 'alias:echo', 'alias:sleep'}
567+
and {'alias:cat', 'alias:echo', 'alias:sleep', 'alias:sort'}
568568
or {'alias:echo'}, ',')..';'
569569
module.exec([[
570570
let &shell = ']]..shell..[['
571571
set shellquote= shellxquote=
572572
let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command ]]..cmd..[['
573573
let &shellpipe = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
574-
let &shellredir = '-RedirectStandardOutput %s -NoNewWindow -Wait'
574+
let &shellredir = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
575575
]])
576576
return found
577577
end

‎test/functional/vimscript/system_spec.lua

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -642,12 +642,12 @@ describe('shell :!', function()
642642
if iswin() then
643643
feed(':4verbose %!sort /R<cr>')
644644
screen:expect{
645-
any=[[Executing command: .?Start%-Process sort %-ArgumentList "/R" %-RedirectStandardInput .* %-RedirectStandardOutput .* %-NoNewWindow %-Wait]]
645+
any=[[Executing command: .?& { Get%-Content .* | & sort /R } 2>&1 | Out%-File %-Encoding UTF8 .*; exit $LastExitCode"]]
646646
}
647647
else
648648
feed(':4verbose %!sort -r<cr>')
649649
screen:expect{
650-
any=[[Executing command: .?Start%-Process sort %-ArgumentList "%-r" %-RedirectStandardInput .* %-RedirectStandardOutput .* %-NoNewWindow %-Wait]]
650+
any=[[Executing command: .?& { Get%-Content .* | & sort %-r } 2>&1 | Out%-File %-Encoding UTF8 .*; exit $LastExitCode"]]
651651
}
652652
end
653653
feed('<CR>')

0 commit comments

Comments
 (0)
0