Fix async direnv on Vim 8.0 #4
Conversation
With the version of Vim 8.0 I have (MacVim, 8.0, patches 1-1272), I am getting this error with direnv.vim: Error detected while processing function .../direnv.vim/plugin/direnv.vim|60| direnv#export[14] E475: Invalid argument: stderr It appears that in Vim 8.0, the dictionary provided to the job must not have any keys *except* the callback keys, unlike with neovim. After several experimental implementations, this is the one that works.
| \ } | ||
| endif | ||
|
|
||
| function! direnv#export() abort |
There was a problem hiding this comment.
Splitting this function to autoload folder makes no sense as it is launched on VimEnter.
There was a problem hiding this comment.
It is launched on VimEnter, but it requires access to s:job_status and s:direnv_cmd, which it won’t have if it remains in plugin/direnv.vim. It wouldn’t be hard to rewrite this to just use s: functions, but given that direnv#export() is available, it is typical in the VimL that I’ve seen to put functions defined that way in autoload/.
| let s:direnv_auto = get(g:, 'direnv_auto', 1) | ||
| let s:job_status = { 'running': 0, 'stdout': [''], 'stderr': [''] } | ||
|
|
||
| function! direnv#auto() abort |
There was a problem hiding this comment.
Moving this function to autoload makes no sense as it is launched always when sourcing plugin/direnv.vim.
There was a problem hiding this comment.
I only moved this because of line 8 (the moving of let s:direnv_auto); it’s probably not necessary to move.
| " Author: zimbatm <http://zimbatm.com/> & Hauleth <[email protected]> | ||
| " Version: 0.3 | ||
|
|
||
| scriptencoding utf-8 |
There was a problem hiding this comment.
No need for that, as there are no non-ASCII characters in this file.
There was a problem hiding this comment.
I’ve generally taken the habit of putting that in there all the time. It’s not necessary, but it shouldn’t hurt.
|
Ok, I will fix this issue later. |
| let s:direnv_auto = get(g:, 'direnv_auto', 1) | ||
| let s:job_status = { 'running': 0, 'stdout': [''], 'stderr': [''] } | ||
|
|
||
| function! direnv#auto() abort |
There was a problem hiding this comment.
I only moved this because of line 8 (the moving of let s:direnv_auto); it’s probably not necessary to move.
| \ } | ||
| endif | ||
|
|
||
| function! direnv#export() abort |
There was a problem hiding this comment.
It is launched on VimEnter, but it requires access to s:job_status and s:direnv_cmd, which it won’t have if it remains in plugin/direnv.vim. It wouldn’t be hard to rewrite this to just use s: functions, but given that direnv#export() is available, it is typical in the VimL that I’ve seen to put functions defined that way in autoload/.
| call delete(l:tmp) | ||
| endif | ||
| endfunc | ||
| " MacVim (vim 8.0) with patches 1-1272 throws an error if a job option is given |
There was a problem hiding this comment.
This comment is not necessary, as it describes an earlier attempt, where I attempted to have s:job_status (the way that it is defined that works for neovim) be assigned to s:job on neovim and then have a simple s:job with the necessary callback that forwards to the functions on s:job_status. I couldn’t get that working at all (a lot of errors about dictionary function called with no dictionary, but apparently in the async job handler called by job_start()). Thus, the move to separate functions from callbacks entirely.
| endfunction | ||
|
|
||
| function! direnv#on_stdout(_, data, ...) abort | ||
| let s:job_status.stdout = a:data |
There was a problem hiding this comment.
With the previous form of this (let s:job_status.stdout[-1] = a:data[0]; call extend(s:job_status.stdout, a:data[1])) I was getting out-of-range errors if the output was empty (particularly with the .stderr version).
|
@halostatue could you please check my fix on #6? |
|
@zimbatm Ping, it's been 11 days. |
|
There’s some back and forth on possible fixes that are less invasive in #6, but I haven’t seen movement in that in a couple days (and the base fixes in #6 do not fix all of the issues, but I don’t know if the array/extend trick is an nvim optimization or not) @zimbatim asked for assistance with CI (#5?) that I’ve gad no time to help with and I don’t use nvim (no good Mac GUI yet). I’m not sure whether I’m running this or my modified version of #6, but both work beautifully for me so MacVim in both modes. |
|
I'm just going to merge this since @halostatue hasn't come back to us |
With the version of Vim 8.0 I have (MacVim, 8.0, patches 1-1272), I am getting this error with direnv.vim:
It appears that in Vim 8.0, the dictionary provided to the job must not have any keys except the callback keys, unlike with neovim.
After several experimental implementations, this is the one that works.