steelbrain / linter

A Base Linter with Cow Powers http://steelbrain.me/linter/
MIT License
1.1k stars 178 forks source link

Add Promise-based suggestions #1519

Open cdavidjean opened 7 years ago

cdavidjean commented 7 years ago

I'm modifying the dartlang plugin, and the way dart analyzer works is it computed possible fixes on demand, asynchronously (they are expensive to compute and it wouldn't be friendly if you have a long error list). So I have to give a suggestion that says 'Quick Fix for -an error-' which triggers computing fixes and offering another listbox to the user.

If we could have a Promised based suggestion then user could skip the intermediate listbox. i.e., something like:

export type Message = {
  // Automatically added
  key: string,
  ...
  solutions?: Array<{
    title?: string,
    position: Range,
    priority?: number,
    currentText?: string,
    replaceWith: string,
  } | {
    title?: string,
    priority?: number,
    position: Range,
    apply: (() => any),
  }> **| (() => Promise<{
    title?: string,
    priority?: number,
    position: Range,
    apply: (() => any),
  }>)**,
  ...
}
Arcanemagus commented 7 years ago

It sounds like you don't know before hand whether a solution will even be possible, the UI needs to know right away though in order to properly display the message. How do you propose working around that?

It sounds like it might be better to move your provider to the "Indie Linter" API, where you can push messages to Linter at any time. That way you can simply hold off on sending those messages that you are proposing be Promises, and instead when they resolve internally just send them to Linter then.

cdavidjean commented 7 years ago

I do know beforehand because the analyzer tells me 'hasFix' for each error. Turning around and asking the analyzer every quick fix for every error sounds expensive and slow.
By the way, I am using the indie linter (hopefully the right way ;). Thanks

steelbrain commented 7 years ago

Hi @cdavidjean

The way I see it, you should have a logic like this in the code

const errorFromCompiler = getErrorMagically()
const solution = {
  ...
  async apply(){
    const solution = await getSolutionMagically(errorFromCompiler)
    ...
  } 
  ...
}

So you can indicate you have a solution but the callback will only ever be invoked if the user opts for it

cdavidjean commented 7 years ago

Hi @steelbrain, That is what I would like to do, but AFAIK apply() in solution isn't async. My idea is also that the most of the intentions (quick fixes) are not necessarily 'alt-entered' by the user, so why waste cycles computing all those solutions if they are not needed/wanted. Thanks

steelbrain commented 7 years ago

@cdavidjean It can be async, you can do whatever you want in there!

cdavidjean commented 7 years ago

Sorry, I should have check my code before answering. What you suggest is what I do which leads to two level of listboxes to get to quick fix. Each Message has one solution that says 'Quick Fix', and when apply() is called I get the quick fixed async and then show them in a custom listbox instead of the intentions UI.

What I should've suggested in the third form of solutions above is (added Array):

...| (() => Promise<Array<{ title?: string, priority?: number, position: Range, apply: (() => any), }>>)

So that computing solutions for a Message can be async and deferred to when user triggers intentions (quick fixes)

Arcanemagus commented 7 years ago

@cdavidjean If the user has requested the fix, why not just apply it when the request resolves?

cdavidjean commented 7 years ago

They get a choice of fixes, not a single fix.