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

Implement a uniform API for asynchronous processing for most ALE features #2132

Closed
12 tasks done
w0rp opened this issue Dec 9, 2018 · 7 comments
Closed
12 tasks done

Comments

@w0rp
Copy link
Member

w0rp commented Dec 9, 2018

Problem description

This issue outlines a design for how to add more asynchronous execution support
to ALE's codebase. Currently, ALE can run the following in the background.

  • Jobs for getting linting results. (callback, command_callback)
  • Jobs for figuring out how to build the commands for jobs. (command_chain)
  • Jobs for fixing files. (:ALEFix functions)
  • Jobs for chaining fixing files (chain_with)
  • Sending messages to LSP servers or tsserver, etc.

ALE does not support asynchronous execution in the following common scenarios, at the time this issue was written.

  • Figuring out how to build an executable path for a linter. (You can do this for a fixer at the moment, but not a linter.)
  • Figuring out how to build a command to run for a language server. (command_chain isn't currently implemented for LSP commands.)
  • Running additional jobs when parsing linter results.

If ALE did add support for all the above, it would be possible to improve on the user experience in the following ways.

  • ALE could discover valid execution paths for running linters in the background, so I/O never blocks editing in Vim. This isn't a problem for most people right now, but it is a problem for some people, and that makes it a problem.
  • ALE could support --version checks and more for LSP commands.
  • ALE could support sending the results of one linter to another program for additional processing, where that program might be able to improve on the output in the background, without blocking Vim.
  • There will be more uses I can't imagine, and that is a good thing.

Proposal

All of the problems above can be solved with a uniform API for signaling when results are ready with a callback. A new function will be added which returns a special Dictionary representing a deferred object and accepts a callback. The callbacks for linters, fixers, and more will be able to return one of these deferred objects to indicate that something will be run in the background, and the result of the callback will be used as the eventual result.

Here is just one example of how this API could be used.

" This example is partly adapted from what the DMD linter does today.

function! s:ParseDubResults(buffer, output, metadata) abort
  " Do something with a:output here.

  return 'dmd ...'
endfunction

function! ale_linters#d#dmd#Command(buffer) abort
  " The result of this function is a deferred object that will be given to the 
  " ALE engine code. The callback below will be called later and will return
  " the eventual result.
  return ale#command#Run(
  \ a:buffer, 
  \ '... && dub describe --import-paths',
  \ function('s:ParseDubResults'),
  \)
endfunction

The _callback forms of options for ale#linter#Define will be deprecated and
later removed, and command_chain will be deprecated and removed now it can
be replaced with the uniform API.

Deprecation process

There will be a long deprecation process, in the following stages.

  • Support for the uniform API and changes to options in 2.4.0.
  • At some point later, add warnings for old ways of doing things that will be deprecated. Time TBD
  • Much later on, remove all support for the old ways of doing things, and release ALE 3.0.0. Time TBD.

Rationale

I will post the questions I can imagine regarding the coming implementation of this feature, and the answers I have already formulated.

Why the long deprecation process?

Thousands of people use ALE, and immediately removing support for just about anything will make a lot of people angry. Someone or other is likely to be angry when a feature is eventually removed anyway, but at least they'll be given time to update their code. Removing support for the older ways of doing things is necessary for keeping the codebase small and easy to maintain, and therefore
less prone to bugs in the future.

Why not use Promises/Futures for this?

Callbacks typically lead to better performance than promises. See here and here.

I find promises are a much cleaner API for implementing asynchronous tasks, and they are best used when a language already implements async ... await or will eventually implement async ... await. I do not believe that async ... await will ever be implemented in VimL.

Promises offer a failure mechanism like .catch in JavaScript, but there's no need for this for what we need for ALE. Our callbacks will only need to either respond with a result, which could be a result for cancelling a request, never an exception or other type of error.

Why not rewrite the codebase in something other than VimL?

Because the only thing that's near enough to being as portable as VimL is Python code, and near total portability is a strongly desired design goal for ALE. Usage of ALE since its inception has proven the "only VimL" rule to be a good decision, and worth all of the effort.

Features to be deprecated

Feature Replacement
ale#engine#CreateDirectory ale#command#CreateDirectory
ale#engine#CreateFile ale#command#CreateFile
ale#engine#EscapeCommandPart ale#command#EscapeCommandPart
ale#engine#ManageDirectory ale#command#ManageDirectory
ale#engine#ManageFile ale#command#ManageFile
command_chain for linters Use command and new async tools instead
command_callback for linters Use command with a Funcref instead
executable_callback for linters Use executable with a Funcref instead
address_callback for LSP linters Use new address with a Funcref instead
language_callback for LSP linters Use language with a Funcref instead
project_root_callback for LSP linters Use new project_root with a Funcref instead
initialization_options_callback for LSP linters Use initialization_options with a Funcref instead
lsp_config_callback for LSP linters Use lsp_config with a Funcref instead
chain_with for fixers Use new async tools instead

To Do

Here is what remains to be done. The list will be extended as time goes on.

  • Unify current command and temporary file handling in command.vim.
  • Copy old engine.vim functions to command.vim.
  • Add an ale#job function for just running a command and getting the output in one List. Use it in engine.vim.
  • Implement deferred exectuable results for linters.
  • Implement deferred command results for linters.
  • Implement deferred exectuable results for language servers.
  • Implement deferred command results for language servers.
  • Implement deferred results for fixers.
  • Document ale#command#Run and how to use it.
  • Add deprecation warnings for old engine.vim functions to be removed later. (ManageDirectory, etc.)
  • Add deprecation warnings for _callback linter options and command_chain.
  • Add deprecation warnings for chain_with for fixers.
@andreypopp
Copy link
Contributor

I think it would be useful to make all async functions use promise like
abstraction.

Motivation:

  • We can keep the old signature for callbacks (buffer, lines) but handle the
    return value being a promise.

  • Promises makes it easier to enforce that some async value should be computed
    once:

    function! s:getCapabilities(linter) abort
      if a:linter.capabilities is v:null
        " s:doGetCapabilities() returns a promise
        let a:linter.capabilities = s:doGetCapabilities(a:linter)
      endif
    
      return a:linter.capabilities
    endfunction
    

    Same for getting current outline per buffer (for example) from LSP (can be
    invalidated by other code by just replacing the corresponding variable by the
    new promise which does the work).

  • Promises are easier to compose than callbacks.

    For example, to combine multiple results from different LSP servers:

    Promise.all([req1, req2, ...])
    

We can vendor something like Async.Promise as ale#promise which provides
JavaScript-like promise implementation for VimL.

@w0rp
Copy link
Member Author

w0rp commented Jan 22, 2019

See the section "Why not use Promises/Futures for this." If this was JavaScript, yes. This is Vim script, so that won't happen.

@andreypopp
Copy link
Contributor

andreypopp commented Jan 22, 2019

I don't buy performance argument as the overhead would be negligible compared to I/O. Still the section doesn't list the pros I listed in the comment above.

@w0rp
Copy link
Member Author

w0rp commented Jan 22, 2019

The performance will definitely be worse because of the additional logic required for an implementation of promises in Vim script. We don't need the exception handling mechanisms. We'll be using callbacks.

w0rp added a commit that referenced this issue Feb 6, 2019
A new function is added here which will later be modified for public use
in linter and fixer callbacks. All linting and fixing now goes through
this new function, to prove that it works in all cases.
@w0rp
Copy link
Member Author

w0rp commented Feb 6, 2019

The new function will probably eventually be used like so.

function! ChainedCallback(Done, buffer, output, data) abort
    call a:Done('second command')
endfunction

function! CommandCallback(buffer, Done) abort
    return ale#command#Run(a:buffer, 'first command', {
    \   'callback': function('ChainedCallback', [a:Done]),
    \})
endfunction

The callback is given (buffer, output, metadata) as arguments, and ale#command#Run returns v:true when a job is started and v:false when a job fails. That way, you can just return the result from a linter or fixer callback, and ALE will know to wait for done.

@w0rp
Copy link
Member Author

w0rp commented Feb 8, 2019

With a little bit of tweaking, it can be more like this.

function! ChainedCallback(buffer, output, data) abort
    return 'second command'
endfunction

function! CommandCallback(buffer) abort
    return ale#command#Run(a:buffer, 'first command', 'ChainedCallback')
endfunction

The result of Run is replaced with a Dictionary representing a deferred object, and the done function is assigned to that. This should be a lot easier to use in general.

This will make the primary use case for this, finding executables to run in the background, a lot easier.

function! ExecutableCallback(buffer) abort
    " The deferred object is returned here.
    return ale#node#FindExecutable(
    \   a:buffer, 
    \   'javascript_flow_ls', 
    \   ['node_modules/.bin/flow']
    \)
endfunction

@w0rp
Copy link
Member Author

w0rp commented Feb 22, 2019

I reverted the changes for adding the done function for fixers now the design has changed, and I updated the information above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants