neomake / neomake

Asynchronous linting and make framework for Neovim/Vim
MIT License
2.66k stars 368 forks source link

Integrating custom maker for language plugin #1109

Open mhartington opened 7 years ago

mhartington commented 7 years ago

Expected behavior

Hi, I am the author of nvim-typescript and I'm working on replacing the built in typescript tsc maker, with one that uses the typescript client.

nvim-geterr

Currently, it's called through it's own command TSGetErr but would love to add a custom makers that integrates with neomake.

What would be the recommended way to handle this? The command is written in python and calls a ts-client that returns some raw output.

https://gist.github.com/mhartington/94b2940c77a8f5b619b8439d20c4c76f

I will extract this out into a function through the neovim-function decorator, but is there a return value that neomake expect?

Similar to how tsuquyomi integrates with syntastic. https://github.com/Quramy/tsuquyomi/blob/master/syntax_checkers/typescript/tsuquyomi.vim

blueyed commented 7 years ago

I think you should wait for https://github.com/neomake/neomake/pull/1050. Basically you would define a get_list_entries method for the maker, that returns a list of entries that are then processed by Neomake, without starting a job. I might look into getting this in later tonight, otherwise during the next days. Given that we also can handle JSON by now, this allowed me to write a redpen-server maker, which uses curl - this was the reason for me to add auto-linting etc.. (with tempfile handling (just merged)).

mhartington commented 7 years ago

Alright, and the list of entries would just match something from a quickfix entry?

As for handling json, that would be even better, since the return value from the ts-client is just json

{
  'seq': 0,
  'type': 'event',
  'event': 'semanticDiag',
  'body': {
    'file': '/Users/mhartington/myApp/src/pages/home/home.ts',
    'diagnostics': [

    {
      'start': { 'line': 7, 'offset': 3},
      'end': {'line': 7,'offset': 12},
      'text': 'Module \'"/Users/mhartington/myApp/node_modules/@angular/core/index"\' has no exportedmember \'component\'.'
    },

    {
      'start': {'line': 15,'offset': 2},
      'end': {'line': 15, 'offset': 11},
      'text': "Cannot find name 'Component'."
    }]
  }
}

I'm just processing it and creating a quickfix list item currently, but if I can avoid that, that would be great!

ahstro commented 7 years ago

@mhartington FYI, #1050 is merged :)

mhartington commented 7 years ago

Currently in the works of adding this. But one thing I noticed is that the function being called in get_list_entries has to be synchronous. Going to do some more investigation, but it looks like it's not possible to pass it an async function (though python) and return the value

mhartington commented 7 years ago

So did some thinking about how to handle this. Currently, if you use get_list_entries it expects a return value, which may not happen right away if the plugin is async. Asking around on gitter, an idea came up would be to pass the results of the async function to some other callback.

So I was thinking, what if there was a new key and a new callback function in neovim:


let g:neomake_typescript_nvim_maker = { 'is_async': 1 }
function! g:neomake_typescript_nvim_maker.get_list_entries(jobinfo) abort
  call TSGetErrFunc(a:jobinfo)
endfunction

This in turn, calls the python/async function.

@neovim.function("TSGetErrFunc")
def getErrFunc(self, args=None):
    getErrRes = self._client.getErr([self.relative_file()])
    self.vim.call('neomake#process_remote_job', args[0], getErrRes )

Since the last bit of the function is a synchronous vim call, it will be milliseconds of delay, not awful.

Then a new call back function, neomake#process_remote_jobwill take the jobinfo and results, can call s:ProcessEntries in autoload/neomake.vim

I have feeling this approach could work for #1361 as well, since tsuquyomi does use jobs (in vim8 only though).

@blueyed let me know what you think, I can try to put together a PR in a few days, but would like to hear your thoughts/opinion on this. I personally would love to have this 😄

blueyed commented 7 years ago

@mhartington With https://github.com/neomake/neomake/pull/1370 (where I have local changes also) there will be support for delaying/retrying functions/processing in general. This is used there for when callbacks happen while in a 'tabline' function etc, but this concept could be used here.

What about having a separate get_list_entries function (e.g. get_list_entries_async), or make the async variant return something special, or set something on jobinfo? If it would return something like neomake#async_entries (which would be a dict, and could be checked with is), Neomake could handle this job as being currently run, until the async function would call e.g. jobinfo.add_async_entries(), passing in its results. I will give this a try.

mhartington commented 7 years ago

I like that approach!

blueyed commented 7 years ago

See https://github.com/neomake/neomake/pull/1399.