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 async direnv on Vim 8.0 #4

Merged
merged 1 commit into from
Dec 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions autoload/direnv.vim
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
" direnv.vim - support for direnv <http://direnv.net>
" Author: zimbatm <http://zimbatm.com/> & Hauleth <[email protected]>
" Version: 0.3

scriptencoding utf-8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for that, as there are no non-ASCII characters in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve generally taken the habit of putting that in there all the time. It’s not necessary, but it shouldn’t hurt.


let s:direnv_cmd = get(g:, 'direnv_cmd', 'direnv')
let s:direnv_auto = get(g:, 'direnv_auto', 1)
let s:job_status = { 'running': 0, 'stdout': [''], 'stderr': [''] }

function! direnv#auto() abort
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this function to autoload makes no sense as it is launched always when sourcing plugin/direnv.vim.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only moved this because of line 8 (the moving of let s:direnv_auto); it’s probably not necessary to move.

return s:direnv_auto
endfunction

function! direnv#on_stdout(_, data, ...) abort
let s:job_status.stdout = a:data
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

endfunction

function! direnv#on_stderr(_, data, ...) abort
let s:job_status.stderr = a:data
endfunction

function! direnv#on_exit(_, status, ...) abort
let s:job_status.running = 0

for l:m in s:job_status.stderr
if l:m isnot# ''
echom l:m
endif
endfor
exec join(s:job_status.stdout, "\n")
endfunction

function! direnv#err_cb(_, data) abort
call direnv#on_stderr(0, split(a:data, "\n", 1))
endfunction

function! direnv#out_cb(_, data) abort
call direnv#on_stdout(0, split(a:data, "\n", 1))
endfunction

function! direnv#exit_cb(_, status) abort
call direnv#on_exit(0, a:status)
endfunction

if has('nvim')
let s:job =
\ {
\ 'on_stdout': 'direnv#on_stdout',
\ 'on_stderr': 'direnv#on_stderr',
\ 'on_exit': 'direnv#on_exit'
\ }
else
let s:job =
\ {
\ 'out_cb': 'direnv#out_cb',
\ 'err_cb': 'direnv#err_cb',
\ 'exit_cb': 'direnv#exit_cb'
\ }
endif

function! direnv#export() abort
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Splitting this function to autoload folder makes no sense as it is launched on VimEnter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/.

if !executable(s:direnv_cmd)
echoerr 'No Direnv executable, add it to your PATH or set correct g:direnv_cmd'
return
endif
if s:job_status.running
return
endif

let s:job_status.running = 1
let l:cmd = [s:direnv_cmd, 'export', 'vim']
if has('nvim')
call jobstart(l:cmd, s:job)
elseif has('job') && has('channel')
call job_start(l:cmd, s:job)
else
let l:tmp = tempname()
echom system(printf(join(l:cmd).' '.&shellredir, l:tmp))
exe 'source '.l:tmp
call delete(l:tmp)
endif
endfunction
70 changes: 9 additions & 61 deletions plugin/direnv.vim
Original file line number Diff line number Diff line change
Expand Up @@ -2,78 +2,26 @@
" Author: zimbatm <http://zimbatm.com/> & Hauleth <[email protected]>
" Version: 0.2

if exists('g:loaded_direnv') || &cp || v:version < 700
if exists('g:loaded_direnv') || &compatible || v:version < 700
finish
endif
let g:loaded_direnv = 1

let s:direnv_cmd = get(g:, 'direnv_cmd', 'direnv')
let s:direnv_auto = get(g:, 'direnv_auto', 1)

let s:job = { 'running': 0, 'stdout': [''], 'stderr': [''] }
" NeoVim {{{
func! s:job.on_stdout(_, data, ...) abort
let self.stdout[-1] .= a:data[0]
call extend(self.stdout, a:data[1:])
endfunc
func! s:job.on_stderr(_, data, ...) abort
let self.stderr[-1] .= a:data[0]
call extend(self.stderr, a:data[1:])
endfunc
func! s:job.on_exit(_, status, ...) abort
let self.running = 0

for m in self.stderr
if m isnot# ''
echom m
endif
endfor
exe join(self.stdout, "\n")
endfunc
" }}}
" Vim {{{
func! s:job.err_cb(_, data) abort
call self.on_stderr(0, split(a:data, "\n", 1))
endfunc
func! s:job.out_cb(_, data) abort
call self.on_stdout(0, split(a:data, "\n", 1))
endfunc
func! s:job.exit_cb(_, status) abort
call self.on_exit(0, a:status)
endfunc
" }}}

func! direnv#export() abort
if !executable(s:direnv_cmd)
echoerr 'No Direnv executable, add it to your PATH or set correct g:direnv_cmd'
return
endif
if s:job.running
return
endif

let s:job.running = 1
let l:cmd = [s:direnv_cmd, 'export', 'vim']
if has('nvim')
call jobstart(l:cmd, s:job)
elseif has('job') && has('channel')
call job_start(l:cmd, s:job)
else
let l:tmp = tempname()
echom system(printf(join(l:cmd).' '.&shellredir, l:tmp))
exe 'source '.l:tmp
call delete(l:tmp)
endif
endfunc
" MacVim (vim 8.0) with patches 1-1272 throws an error if a job option is given
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

" extra fields that it does not recognize. If the job ran even with the error
" message, this could be fixed with `silent!`, but the job doesn't run.
"
" To fix this, we give `vim` an empty `s:job` dictionary that calls back to the
" `s:job_status` dictionary. `nvim` gets `s:job` set as `s:job_status`.

command! -nargs=0 DirenvExport call direnv#export()

augroup envrc
augroup direnv_rc
au!
autocmd BufRead,BufNewFile .envrc set filetype=sh
autocmd BufWritePost .envrc DirenvExport

if s:direnv_auto
if direnv#auto()
autocmd VimEnter * DirenvExport

if exists('##DirChanged')
Expand Down