Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not assume shell for platform #254

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Sh3Rm4n
Copy link

@Sh3Rm4n Sh3Rm4n commented Oct 10, 2021

win32 does not necessarily mean cmd.exe as well as others does not mean sh.

Problem is, that previous functions, like s:escape_cword() do respect shell e.g.
with shellescape(), but passing the arguments to job_start() / jobstart()
does ignore shell.

This leads to trouble. E.g. I use fish, where shellescape() escapes \, which means
if <cword> / -cword is used (e.g. Aword), s:escape_cword() results to \\bAword\\b
(which is correct), but this string is then passed to jobstart() executed by
sh, which does not share the same escaping rules.

win32 does not necessarily mean `cmd.exe`
as well as others does not mean `sh`.

Problem is, that previous functions, like
s:escape_cword() do respect `shell` e.g.
with `shellescape()`, but passing
the arguments to `job_start()` / jobstart()
does ignore `shell`.

This leads to trouble. E.g. I use `fish`,
where `shellescape()` escapes `\`, which means
if <cword> / -cword is used (e.g. `Aword`),
s:escape_cword() results to `\\bAword\\b`
(which is correct), but this string
is then passed to `jobstart()` executed by
`sh`, which does not share the same escaping rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant