Skip to content

Add async support for neovim#81

Merged
mitermayer merged 1 commit intoprettier:feature/adding-support-for-neovim-asyncfrom
neoclide:nvim-job
Dec 29, 2017
Merged

Add async support for neovim#81
mitermayer merged 1 commit intoprettier:feature/adding-support-for-neovim-asyncfrom
neoclide:nvim-job

Conversation

@chemzqm
Copy link
Copy Markdown
Contributor

@chemzqm chemzqm commented Nov 15, 2017

Using job control for async job when using neovim.

@mitermayer
Copy link
Copy Markdown
Member

Thanks so much for that @chemzqm !!! Super happy to see this!

Comment thread autoload/prettier.vim

function! s:Prettier_Job_Nvim_Exit(status, info, out, err)
if a:status != 0
echoerr join(a:err, "\n")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we instead of echo the error forward the call to s:Prettier_Job_Error(msg) that is responsible for printing the error and also handling synthax error lines to the quickfix.

An example of trying to parse an invalid file and having the quickfix opening on the line number:

output

Comment thread autoload/prettier.vim
let l:dict = {
\ 'start': a:startSelection - 1,
\ 'end': a:endSelection,
\ 'buf_nr': bufnr('%'),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we store the buffer name before we start the job call ? If we don't do that the user can save the file and move to a different buffer and get its contents overwritten with the output of this job execution, refer to : #62

Comment thread autoload/prettier.vim
\}
let l:out = []
let l:err = []

Copy link
Copy Markdown
Member

@mitermayer mitermayer Nov 15, 2017

Choose a reason for hiding this comment

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

can we shield this with a flag to make sure that only a single job can be running at a time similar how we do for vim8 async ?

  if s:prettier_job_running != 1
      let s:prettier_job_running = 1
      let l:job = jobstart(l:async_cmd, {
      \ 'on_stdout': {job_id, data, event -> extend(l:out, data)},
      \ 'on_stderr': {job_id, data, event -> extend(l:err, data)},
      \ 'on_exit': {job_id, status, event -> s:Prettier_Job_Nvim_Exit(status, l:dict, l:out, l:err)},
      \ })
    call jobsend(l:job, l:lines)
     call jobclose(l:job, 'stdin')
  endif

Comment thread autoload/prettier.vim
endfunction

function! s:Prettier_Job_Nvim_Exit(status, info, out, err)
if a:status != 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Assuming we did add the running flag here we unset it

function! s:Prettier_Job_Nvim_Exit(status, info, out, err)
    let s:prettier_job_running = 0
    if a:status != 0

Comment thread autoload/prettier.vim
if len(a:out) == 0 | return | endif

let l:last = a:out[len(a:out) - 1]
let l:out = l:last == '' ? a:out[0:len(a:out) - 2] : a:out
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is this line doing ? could we add some comments before it ?

@mitermayer
Copy link
Copy Markdown
Member

Once again thank you so much for submitting this PR, it has been one of the things I wanted to do before 1.0!! wohooo.

Added some comments to it, havent tested it yet but it looks good!

@mitermayer
Copy link
Copy Markdown
Member

I have been pretty busy lately but will get back in this PR next week and make sure it will get merged in some form!

@lewisflude
Copy link
Copy Markdown

Hey @mitermayer! Could this get merged in soon?

@mitermayer
Copy link
Copy Markdown
Member

Sorry guys, will get into this ASAP, added some comments in the PR that was hoping to get addressed, but if they don’t I will address myself

@lewisflude
Copy link
Copy Markdown

@mitermayer That's alright! Whenever works! :)

@mitermayer mitermayer changed the base branch from master to feature/adding-support-for-neovim-async December 29, 2017 19:59
@mitermayer mitermayer merged commit dff49bc into prettier:feature/adding-support-for-neovim-async Dec 29, 2017
@mitermayer
Copy link
Copy Markdown
Member

mitermayer commented Dec 29, 2017

Merged this on branch feature/adding-support-for-neovim-async where I will be addressing some of the previous comments in order to merge it to master.

Thanks a lot for the work on this PR

@lewisflude
Copy link
Copy Markdown

Thanks @mitermayer!

@chemzqm chemzqm deleted the nvim-job branch January 15, 2018 04:15
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.

3 participants