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

nvim binary: use 'vim.v.progpath' instead of 'vim.v.argv[1]' #47

Merged
merged 1 commit into from
Jul 2, 2022
Merged

Conversation

ibhagwan
Copy link
Contributor

When the user opens neovim using a relative path and then uses :cd to change the working directory the neovim binary path (used in the shell helper) will be have the wrong path and fail to run.

I encountered this in fzf-lua when using the nightly binary tar and opening neovim with ./nvim/... after using cd once the path provided by vim.v.argv[1] no longer works.

Using v:progpath is safer and always provides the full path of the neovim binary.

@vijaymarupudi vijaymarupudi merged commit ea1df3a into vijaymarupudi:master Jul 2, 2022
@vijaymarupudi
Copy link
Owner

Didn't think of that, thank you!

@ibhagwan
Copy link
Contributor Author

ibhagwan commented Jul 3, 2022

Didn't think of that, thank you!

You're welcome @vijaymarupudi!

Every fix of importance in fzf-lua I also try to PR here, on this note there's another imporvement I added to fzf-lua which might be of interest to you, instead of piping the command into fzf I now use a temporary environment variable FZF_DEFAULT_COMMAND (sent inside the termopen env table), the benefit is twofold:

  • By using the env variable fzf itself spawns the command and will kill it when it exists making for much cleaner and faster exists, sometimes I noticed delays when cancelling running commands, especially when run inside the neovim headless wrapper if I didn't detect the signal and kill it
  • Better windows support (I still don't support windows but still)

You can confirm what I'm saying in junegunn/fzf#1750 (comment):

That's why we're temporarily setting FZF_DEFAULT_COMMAND in the above example. By doing so, fzf launches the process and it can kill it when it exits.

I can quickly PR it for you if you wish, lmk.

@3rd
Copy link

3rd commented Jul 12, 2022

@vijaymarupudi @ibhagwan

I think this broke the plugin for me, when I try to use fzf.fzf() I get Vim:E475: Invalid argument: env now.

I'm on NixOS, and vim.v.progpath looks ok.

It's working fine on e3973c18931e63dc13c5efa8fb0fc3b08f1dc90d.

@ibhagwan
Copy link
Contributor Author

I would assume it’s one of two commits, can you please verify if it’s broken by ea1df3a or b1a537c?

Also when inside neovim can you please run :lua print(vim.v.progpath) and :lua print(vim.v.argv[1]) and post the result here?

@3rd
Copy link

3rd commented Jul 12, 2022

Ah it's actually from b1a537c, I guess from here since it's throwing the Invalid argument: env error.

I'm on NVIM v0.8.0-dev, and I get the same error when running vim.fn.termopen("ls", { env = {}}).
It's weird as env is a documented option and should work.Looks like a Neovim issue.

@ibhagwan
Copy link
Contributor Author

Ah it's actually from b1a537c, I guess from here since it's throwing the Invalid argument: env error.

I'm on NVIM v0.8.0-dev, and I get the same error when running vim.fn.termopen("ls", { env = {}}). It's weird as env is a documented option and should work.Looks like a Neovim issue.

You're correct, I applied the same code from fzf-lua here but in fzf-lua I always send env. { ['SHELL'] = 'sh', ... } so env is never an empty table, I'll submit a PR to fix this.

@ibhagwan
Copy link
Contributor Author

@3rd, let's continue this discussion in #49?

@vijaymarupudi
Copy link
Owner

@3rd should be fixed now

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.

None yet

3 participants