unify async job handling #1864
Merged
unify async job handling #1864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove go#jobcontrol#RemoveHandler, because it is unused.
Remove the desc argument from go#jobcontrol#Spawn, because it is unused.
Set the working directory for Neovim jobs instead of manually changing the directory before starting the job.
arp242
reviewed
Jun 20, 2018
autoload/go/job.vim
Outdated
| @@ -1,7 +1,18 @@ | |||
| " Spawn returns callbacks to be used with job_start. It is abstracted to be | |||
| let s:cd = exists('*haslocaldir') && haslocaldir() ? 'lcd' : 'cd' | |||
There was a problem hiding this comment.
As I understand it, this doesn't do what you want. The haslocaldir() checks if the current window has a local path set already with :lcd. So evaluating it once here isn't really that useful. You need to evaluate it when using.
We could add a wrapper function in util.vim though.
Rename go#job#Spawn to go#job#Options. Add a new key, cwd, to the return value of go#job#Options so that the working directory for the job can be passed in the job options. Add a new function, go#job#Start, to wrap vim8's job_start function. Eventually, this new function will be used to start Neovim jobs, too, so that vim-go can have an abstraction to start jobs that hides the details of dealing with Neovim and Vim8 job API differences.
* Update the status in vim8 so it can be shown on the statusline.
Add go#job#Spawn, a convenience wrapper around go#job#Options and go#job#Start that brings the vim8 and neovim job abstraction closer together.
|
PTAL |
|
@fatih approved the PR in a personal DM. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unify how jobs are handled in
autoload/go/job.vimandautoload/go/jobcontrol.vimso that there's a single abstraction for async jobs instead of separate abstractions for Vim and Neovim. Unifying the abstractions allows us to provide the same behavior around updating status, writing progress, and handling errors in Vim and Neovim.As part of this work, I was able to refactoring async
go testjobs to use the abstraction inautoload/go/job.vimand also enable the tests inautoload/go/test_test.vimfor Neovim.Among other things, this work paves the way to:
job_startorjobstartinto the abstraction so that there are fewer one offs.job_startin Vim, but use a synchronous call in Neovim, to use the abstraction instead.Though this PR is relatively large, the commits that make up this change tell a coherent story using small and focused changes; reviewing this PR will be made easier by reviewing each commit individually.