aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJustin M. Keyes <justinkz@gmail.com>2022-10-01 17:29:15 -0400
committerGitHub <noreply@github.com>2022-10-01 17:29:15 -0400
commit01721aaa667f7fe32f5b9fbf7b6fd99cb775de48 (patch)
tree265b9840b6239cd9107bc9513b3160bd83c7c101
parent618d21fcc9635051a16b67310e51f2729252a46f (diff)
parentf2482b3b533c8ed131a33aaead332c226a9e6aca (diff)
downloadrneovim-01721aaa667f7fe32f5b9fbf7b6fd99cb775de48.tar.gz
rneovim-01721aaa667f7fe32f5b9fbf7b6fd99cb775de48.tar.bz2
rneovim-01721aaa667f7fe32f5b9fbf7b6fd99cb775de48.zip
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: https://github.com/PowerShell/PowerShell/issues/4681
-rw-r--r--runtime/doc/options.txt2
-rw-r--r--src/nvim/ex_cmds.c94
-rw-r--r--test/functional/helpers.lua6
-rw-r--r--test/functional/vimscript/system_spec.lua4
4 files changed, 47 insertions, 59 deletions
diff --git a/runtime/doc/options.txt b/runtime/doc/options.txt
index 836272a60f..5871b82d9c 100644
--- a/runtime/doc/options.txt
+++ b/runtime/doc/options.txt
@@ -5357,7 +5357,7 @@ A jump table for the options with a short description can be found at |Q_op|.
To use PowerShell: >
let &shell = executable('pwsh') ? 'pwsh' : 'powershell'
let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command [Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;'
- let &shellredir = '-RedirectStandardOutput %s -NoNewWindow -Wait'
+ let &shellredir = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
let &shellpipe = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
set shellquote= shellxquote=
diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c
index 462212c4ce..77feddbbf9 100644
--- a/src/nvim/ex_cmds.c
+++ b/src/nvim/ex_cmds.c
@@ -1544,84 +1544,72 @@ char *make_filter_cmd(char *cmd, char *itmp, char *otmp)
size_t len = strlen(cmd) + 1; // At least enough space for cmd + NULL.
- len += is_fish_shell ? sizeof("begin; " "; end") - 1
- : is_pwsh ? sizeof("Start-Process ")
- : sizeof("(" ")") - 1;
+ len += is_fish_shell ? sizeof("begin; " "; end") - 1
+ : !is_pwsh ? sizeof("(" ")") - 1
+ : 0;
if (itmp != NULL) {
- len += is_pwsh ? strlen(itmp) + sizeof(" -RedirectStandardInput ")
+ len += is_pwsh ? strlen(itmp) + sizeof("& { Get-Content " " | & " " }") - 1
: strlen(itmp) + sizeof(" { " " < " " } ") - 1;
}
if (otmp != NULL) {
len += strlen(otmp) + strlen(p_srr) + 2; // two extra spaces (" "),
}
- const char *const cmd_args = strchr(cmd, ' ');
- len += (is_pwsh && cmd_args)
- ? strlen(" -ArgumentList ") + 2 // two extra quotes
- : 0;
-
char *const buf = xmalloc(len);
if (is_pwsh) {
- xstrlcpy(buf, "Start-Process ", len);
- if (cmd_args == NULL) {
- xstrlcat(buf, cmd, len);
+ if (itmp != NULL) {
+ xstrlcpy(buf, "& { Get-Content ", len - 1); // FIXME: should we add "-Encoding utf8"?
+ xstrlcat(buf, (const char *)itmp, len - 1);
+ xstrlcat(buf, " | & ", len - 1); // FIXME: add `&` ourself or leave to user?
+ xstrlcat(buf, cmd, len - 1);
+ xstrlcat(buf, " }", len - 1);
} else {
- xstrlcpy(buf + strlen(buf), cmd, (size_t)(cmd_args - cmd + 1));
- xstrlcat(buf, " -ArgumentList \"", len);
- xstrlcat(buf, cmd_args + 1, len); // +1 to skip the leading space.
- xstrlcat(buf, "\"", len);
+ xstrlcpy(buf, cmd, len - 1);
}
+ } else {
#if defined(UNIX)
// Put delimiters around the command (for concatenated commands) when
// redirecting input and/or output.
- } else if (itmp != NULL || otmp != NULL) {
- char *fmt = is_fish_shell ? "begin; %s; end"
- : "(%s)";
- vim_snprintf(buf, len, fmt, cmd);
-#endif
- // For shells that don't understand braces around commands, at least allow
- // the use of commands in a pipe.
- } else {
- xstrlcpy(buf, cmd, len);
- }
-
-#if defined(UNIX)
- if (itmp != NULL) {
- if (is_pwsh) {
- xstrlcat(buf, " -RedirectStandardInput ", len - 1);
+ if (itmp != NULL || otmp != NULL) {
+ char *fmt = is_fish_shell ? "begin; %s; end"
+ : "(%s)";
+ vim_snprintf(buf, len, fmt, cmd);
} else {
+ xstrlcpy(buf, cmd, len);
+ }
+
+ if (itmp != NULL) {
xstrlcat(buf, " < ", len - 1);
+ xstrlcat(buf, (const char *)itmp, len - 1);
}
- xstrlcat(buf, itmp, len - 1);
- }
#else
- if (itmp != NULL) {
- // If there is a pipe, we have to put the '<' in front of it.
- // Don't do this when 'shellquote' is not empty, otherwise the
- // redirection would be inside the quotes.
- if (*p_shq == NUL) {
- char *const p = find_pipe(buf);
- if (p != NULL) {
- *p = NUL;
+ // For shells that don't understand braces around commands, at least allow
+ // the use of commands in a pipe.
+ xstrlcpy(buf, (char *)cmd, len);
+ if (itmp != NULL) {
+ // If there is a pipe, we have to put the '<' in front of it.
+ // Don't do this when 'shellquote' is not empty, otherwise the
+ // redirection would be inside the quotes.
+ if (*p_shq == NUL) {
+ char *const p = find_pipe(buf);
+ if (p != NULL) {
+ *p = NUL;
+ }
}
- }
- if (is_pwsh) {
- xstrlcat(buf, " -RedirectStandardInput ", len);
- } else {
xstrlcat(buf, " < ", len);
- }
- xstrlcat(buf, itmp, len);
- if (*p_shq == NUL) {
- const char *const p = find_pipe(cmd);
- if (p != NULL) {
- xstrlcat(buf, " ", len - 1); // Insert a space before the '|' for DOS
- xstrlcat(buf, p, len - 1);
+ xstrlcat(buf, (const char *)itmp, len);
+ if (*p_shq == NUL) {
+ const char *const p = find_pipe((const char *)cmd);
+ if (p != NULL) {
+ xstrlcat(buf, " ", len - 1); // Insert a space before the '|' for DOS
+ xstrlcat(buf, p, len - 1);
+ }
}
}
- }
#endif
+ }
if (otmp != NULL) {
append_redir(buf, len, p_srr, otmp);
}
diff --git a/test/functional/helpers.lua b/test/functional/helpers.lua
index 3aec834bea..4fcc190dee 100644
--- a/test/functional/helpers.lua
+++ b/test/functional/helpers.lua
@@ -562,16 +562,16 @@ function module.set_shell_powershell(fake)
assert(found)
end
local shell = found and (iswin() and 'powershell' or 'pwsh') or module.testprg('pwsh-test')
- local set_encoding = '[Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.Encoding]::UTF8;'
+ local set_encoding = '[Console]::InputEncoding=[Console]::OutputEncoding=[System.Text.UTF8Encoding]::new();'
local cmd = set_encoding..'Remove-Item -Force '..table.concat(iswin()
- and {'alias:cat', 'alias:echo', 'alias:sleep'}
+ and {'alias:cat', 'alias:echo', 'alias:sleep', 'alias:sort'}
or {'alias:echo'}, ',')..';'
module.exec([[
let &shell = ']]..shell..[['
set shellquote= shellxquote=
let &shellcmdflag = '-NoLogo -NoProfile -ExecutionPolicy RemoteSigned -Command ]]..cmd..[['
let &shellpipe = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
- let &shellredir = '-RedirectStandardOutput %s -NoNewWindow -Wait'
+ let &shellredir = '2>&1 | Out-File -Encoding UTF8 %s; exit $LastExitCode'
]])
return found
end
diff --git a/test/functional/vimscript/system_spec.lua b/test/functional/vimscript/system_spec.lua
index a778e2f435..ed822add72 100644
--- a/test/functional/vimscript/system_spec.lua
+++ b/test/functional/vimscript/system_spec.lua
@@ -642,12 +642,12 @@ describe('shell :!', function()
if iswin() then
feed(':4verbose %!sort /R<cr>')
screen:expect{
- any=[[Executing command: .?Start%-Process sort %-ArgumentList "/R" %-RedirectStandardInput .* %-RedirectStandardOutput .* %-NoNewWindow %-Wait]]
+ any=[[Executing command: .?& { Get%-Content .* | & sort /R } 2>&1 | Out%-File %-Encoding UTF8 .*; exit $LastExitCode"]]
}
else
feed(':4verbose %!sort -r<cr>')
screen:expect{
- any=[[Executing command: .?Start%-Process sort %-ArgumentList "%-r" %-RedirectStandardInput .* %-RedirectStandardOutput .* %-NoNewWindow %-Wait]]
+ any=[[Executing command: .?& { Get%-Content .* | & sort %-r } 2>&1 | Out%-File %-Encoding UTF8 .*; exit $LastExitCode"]]
}
end
feed('<CR>')