[CLOSED] Expose code inspection providers for the extensions.

[core-ai-bot]

Issue by zaggino
Sunday Sep 08, 2013 at 04:20 GMT
Originally opened as adobe/brackets#5125

---

As mentioned in adobe/brackets#4588 (comment)

API that fetches the active linting provider for a given file entry.

---

zaggino included the following code: https://github.com/adobe/brackets/pull/5125/commits

[core-ai-bot]

Comment by zaggino
Sunday Sep 08, 2013 at 04:34 GMT

---

@peterflynn can you review? i choose rather not to modify the original run function

[core-ai-bot]

Comment by zaggino
Friday Sep 13, 2013 at 13:20 GMT

---

@peterflynn could we do this for sprint 31 please? :)

[core-ai-bot]

Comment by peterflynn
Friday Sep 13, 2013 at 15:12 GMT

---

What would you think of changing the API to something like: inspectFile(fullPath), returning the provider result? This way the provider running code could evolve without clients of this API needing to replicate all those changes. This would also let CodeInspection take care of loading the file from disk (or reusing a Document if open already) instead of each client of the API having to do that.

One thing I have in mind is the possibility of providers becoming async (#5137). An inspectFile() type API would be async right out of the box, so clients wouldn't break if async providers come along later.

[core-ai-bot]

Comment by peterflynn
Friday Sep 13, 2013 at 15:13 GMT

---

@zaggino Done reviewing. Just those two thoughts.

[core-ai-bot]

Comment by zaggino
Friday Sep 13, 2013 at 16:26 GMT

---

@peterflynn Take a look please what do you think. Haven't modified any of the original code yet but this inspectFile() implementation would work for me quite nicely.

[core-ai-bot]

Comment by peterflynn
Friday Sep 13, 2013 at 16:47 GMT

---

@zaggino This is looking like a good direction, yeah.

[core-ai-bot]

Comment by zaggino
Saturday Sep 14, 2013 at 03:58 GMT

---

Done, with a bit of optimization. Original code was multiplying event handlers on the results table with every render. That should be solved now. Please review@peterflynn

[core-ai-bot]

Comment by peterflynn
Monday Sep 16, 2013 at 23:36 GMT

---

@zaggino I haven't been able to give the latest changes a close look yet -- hopefully tomorrow.

But from my quick glance, I'm a little concerned about possible race conditions in run(), e.g. if you switch files before the provider has returned results. Since we're getting close to the end of the sprint and thus a bit more risk-averse, I wonder if the best thing for now would be to take out the support for async providers. That makes it easier to land the API you need before the sprint ends -- and we could introduce async providers next sprint without breaking API compatibility. What do you think?

[core-ai-bot]

Comment by zaggino
Monday Sep 16, 2013 at 23:39 GMT

---

@peterflynn it's not very difficult to check if the checked document is still the current document when the async provider finishes it's work... but I can disable it in this one to be safe and put up another PR with async

[core-ai-bot]

Comment by zaggino
Monday Sep 16, 2013 at 23:41 GMT

---

and right now - unless someone installs an extension with async provider, it won't be used anyway...

[core-ai-bot]

Comment by peterflynn
Tuesday Sep 17, 2013 at 08:55 GMT

---

@zaggino There are other cases too, though -- you might modify & save the file a second time while the first linting is still running, or toggle the panel visibility, or disable linting... We probably shouldn't offer API support until we're sure it will work reliably.

[core-ai-bot]

Comment by zaggino
Tuesday Sep 17, 2013 at 09:41 GMT

---

ok@peterflynn , i'll remove the async parts to keep this simple for now

[core-ai-bot]

Comment by zaggino
Tuesday Sep 17, 2013 at 09:51 GMT

---

Done, inspectFile is still async for cases when the fileEntry is not found in the DocumentManager.getOpenDocumentForPath. Do you think that this is also a problem?

[core-ai-bot]

Comment by zaggino
Monday Sep 23, 2013 at 04:29 GMT

---

@peterflynn Can we merge now? I see Sprint 32 in the master.

[core-ai-bot]

Comment by peterflynn
Wednesday Sep 25, 2013 at 00:48 GMT

---

@zaggino Looking good -- just a few minor comments.