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

[vim] Don't pipe FZF_DEFAULT_COMMAND in Windows #969

Merged
merged 1 commit into from
Jul 9, 2017

Conversation

janlazo
Copy link
Contributor

@janlazo janlazo commented Jul 8, 2017

cmd.exe leaves the pipe hanging for the source command after fzf finishes so powershell and ag keep running and cmd.exe does not close. Putting FZF_DEFAULT_COMMAND in a batchfile hides the issue from cmd.exe so Vim is stuck waiting and Neovim does not run the exit callback. Letting fzf run FZF_DEFAULT_COMMAND passes the responsibility of stopping the source program to fzf executable so Vim/Neovim don't have to wait for the source command to finish executing.

This issue does not happen when running the fzf executable directly in the shell if FZF_DEFAULT_COMMAND is using powershell. ag still gets stuck.

I did not consider ack because it requires perl and is too slow as a default command.

@janlazo
Copy link
Contributor Author

janlazo commented Jul 9, 2017

The external cmd.exe window does not close for gvim and neovim-qt (or any Neovim GUI) because the source command is still running. Running powershell or ag consecutively on large directories slows down the system if they're not killed so they're not viable yet for default commands.

@junegunn
Copy link
Owner

junegunn commented Jul 9, 2017

I see, it was introduced to avoid possible slowness of $SHELL (#552), which should be a non-issue on Windows platform.

@junegunn junegunn merged commit 68bd410 into junegunn:master Jul 9, 2017
@junegunn
Copy link
Owner

junegunn commented Jul 9, 2017

Merged, thanks!

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

Successfully merging this pull request may close these issues.

2 participants