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

Fix Windows support for Unix shells and powershell #860

Merged
merged 11 commits into from
Aug 16, 2019

Conversation

janlazo
Copy link
Contributor

@janlazo janlazo commented Jul 23, 2019

Fix #815
Fix (partial) #690
Fix #831
Fix #832 (issue happens on Windows for custom shell settings without this PR)

@mikelue Can you test if it works?

TODO

  • powershell
    • require set noshellslash, same as cmd.exe
    • fails on do hook after installing the repo (tested with do hook to install the fzf binary)
    • shellredir must be set to | Out-File -Encoding utf8
  • figure out how to port this to fzf Vim plugin (fzf#shellescape uses a different shell default)
  • investigate if Vim escapes executable arguments in job_start([executable, arg1, arg2, ...]) and if it's still broken on cmd.exe, similar to term_start behavior.
    • Still broken in 8.1.1794 so the hack in vim-plug should be ported to fzf

@mikelue
Copy link

mikelue commented Jul 23, 2019

My shellslash is on, which makes s:shellesc(arg) escape '<filename>' for bat file.
The shellescape() function of VIM depends on shellslash.

I think when the script generates the content for bat file. Every thing should be forced for MS-Win no matter what VIM setting is.

I apologize for my special needs(using unix-shell in MS-Win system).

plug.vim Outdated Show resolved Hide resolved
@janlazo
Copy link
Contributor Author

janlazo commented Jul 23, 2019

My shellslash is on, which makes s:shellesc(arg) escape '<filename>' for bat file.
The shellescape() function of VIM depends on shellslash.

I think when the script generates the content for bat file. Every thing should be forced for MS-Win no matter what VIM setting is.

I apologize for my special needs(using unix-shell in MS-Win system).

That's intended. As long as your shell can run the escaped batchfile and PlugInstall and PlugUpdate work, then the patch works.
Within the batchfile, the commands should be escaped for cmd.exe.

@janlazo
Copy link
Contributor Author

janlazo commented Jul 23, 2019

@mikelue Can you test the latest commit?

plug.vim Outdated Show resolved Hide resolved
tempname() is affected by shellslash so expand() is useless.
"/c" flag is required to exec a batchfile.
@janlazo
Copy link
Contributor Author

janlazo commented Aug 3, 2019

It works for me with the following settings

set shell=sh
set shellcmdflag=-c
set shellquote=
set shellslash
let &shellredir = '>%s 2>&1'

if has('quickfix')
  let &shellpipe = '2>&1 | tee'
endif

if v:version >= 704
  set shellxescape=
  let &shellxquote = (!has('nvim') && has('win32')) ? '"' : ''
endif
set shellpipe=\|&\ tee
set shellredir=&>

@mikelue Do these shell options change the behavior of vim-plug compared to what I have? I copied the bash defaults from Vim's :help.

User's shell settings do not affect Vim/Neovim jobs.
plug.vim Outdated Show resolved Hide resolved
plug.vim Show resolved Hide resolved
@janlazo janlazo changed the title Use cmd.exe shellescape inside batchfile Fix support for Unix shells and powershell Aug 4, 2019
@janlazo janlazo changed the title Fix support for Unix shells and powershell Fix Windows support for Unix shells and powershell Aug 4, 2019
Outputs a list containing the batchfile filepath and the new command
to run the batchfile regardless of the user's shell.
@janlazo
Copy link
Contributor Author

janlazo commented Aug 4, 2019

@junegunn Can you review the changes?

plug.vim Outdated Show resolved Hide resolved
plug.vim Show resolved Hide resolved
plug.vim Outdated Show resolved Hide resolved
plug.vim Outdated Show resolved Hide resolved
@mikelue
Copy link

mikelue commented Aug 6, 2019

@mikelue Do these shell options change the behavior of vim-plug compared to what I have? I copied the bash defaults from Vim's :help.

Seems it doesn't change the behavior. But I will change my my shellredir to &>%s.

@janlazo janlazo force-pushed the win/shellescape branch 2 times, most recently from f0eebef to 9208018 Compare August 10, 2019 15:03
- fix cmd.exe shellescape for '%'
- support pwsh (powershell on Linux)
@janlazo
Copy link
Contributor Author

janlazo commented Aug 10, 2019

Why is one job failing? 7.4.52.

@janlazo
Copy link
Contributor Author

janlazo commented Aug 10, 2019

Does vim-plug need to support trusty? It reached EOL this year.

@junegunn
Copy link
Owner

Does vim-plug need to support trusty? It reached EOL this year.

It's nice that we keep things up-to-date, but I wouldn't consider dropping support for older versions of Vim and Linux distros unless there is a good reason, e.g. change required to implement new features, to reduce maintenance burden, etc.

@junegunn junegunn merged commit 8a44109 into junegunn:master Aug 16, 2019
@junegunn
Copy link
Owner

@janlazo Merged, thanks! I'm grateful for your continuous contributions on Windows support. Would it be easier for you if I make you a collaborator of the project? I'm okay with you fixing/closing Windows related issues without my explicit consent.

@janlazo
Copy link
Contributor Author

janlazo commented Aug 16, 2019

@janlazo Merged, thanks! I'm grateful for your continuous contributions on Windows support. Would it be easier for you if I make you a collaborator of the project? I'm okay with you fixing/closing Windows related issues without my explicit consent.

@junegunn Yes, it would be easier. I'm ok with being a collaborator after resolving PRs like #867 for OS support because I don't have a macbook to verify my fixes for macOS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants