diff options
author | Justin M. Keyes <justinkz@gmail.com> | 2022-10-01 17:29:15 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-01 17:29:15 -0400 |
commit | 01721aaa667f7fe32f5b9fbf7b6fd99cb775de48 (patch) | |
tree | 265b9840b6239cd9107bc9513b3160bd83c7c101 | |
parent | 618d21fcc9635051a16b67310e51f2729252a46f (diff) | |
parent | f2482b3b533c8ed131a33aaead332c226a9e6aca (diff) | |
download | rneovim-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.txt | 2 | ||||
-rw-r--r-- | src/nvim/ex_cmds.c | 94 | ||||
-rw-r--r-- | test/functional/helpers.lua | 6 | ||||
-rw-r--r-- | test/functional/vimscript/system_spec.lua | 4 |
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>') |