Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

- modified the problems panel to support multiple code inspector #5235

Closed
wants to merge 2 commits into from

Conversation

ingorichter
Copy link
Contributor

provider for the same language

  • enable/disable the provider via menuitem for every registered provider
  • fixed the JSLint unit tests to account to the new API

@peterflynn: would you mind having a quick look at the changes? Thanks!

  provider for the same language
- enable/disable the provider via menuitem for every registered provider
- fixed the JSLint unit tests to account to the new API
@ghost ghost assigned peterflynn Sep 17, 2013
@peterflynn
Copy link
Member

@ingorichter We're past string freeze, so we won't be able to merge this sprint. I'll give it a look when I'm back in the US next weekend or Monday -- sound ok?

@ingorichter
Copy link
Contributor Author

@peterflynn that's fine with me. I realized too late that there was a string change required to make the messaging better. But this is not mission critical.


if (error.type !== Type.META) {
numProblems++;
if (getProviderState(provider) === true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this guard should be simplified to if (getProviderState(provider)).

Removed the debug/console output.
@ingorichter
Copy link
Contributor Author

No, you have been totally clear on this. I was thinking, that the template can be compiled once, but mustache caches all this information already and I don't have to do this myself. Do you think that we really have to/should do this, if mustache already does it?

@iwehrman
Copy link
Contributor

Huh, I didn't realize that Mustache cached compiled templates, but it sure does. I think if it were me, I'd still only call compile once, but that's just for aesthetic reasons. Carry on as you wish!

@ingorichter
Copy link
Contributor Author

@peterflynn do you have some time to look at this PR?

@peterflynn
Copy link
Member

@ingorichter Fyi, there's some discussion of the UI part of this going on here: https://groups.google.com/forum/#!topic/brackets-dev/Kh83l2eFugI

@diegoleme
Copy link
Contributor

Can I help you with something @ingorichter

@@ -1,11 +1,16 @@
<table class="bottom-panel-table table table-striped table-condensed row-highlight">
<tbody>
{{#reportList}}
<tr class="inspector-section" data-provider="{{providerName}}">
<td colspan="3"><span class="disclosure-triangle expanded"></span>{{providerName}} ({{numProblems}})</td>
</tr>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it's better to interleave the results so they're just sorted together by line number?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and then add a new column to show the linter name -- maybe hidden if there's only one linter enabled for a given filetype)

@peterflynn
Copy link
Member

@ingorichter Sorry it took me so long to give this a good look :-( Hope this feedback is still helpful.

@ingorichter
Copy link
Contributor Author

Closed in favor of this #5935

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

Successfully merging this pull request may close these issues.

4 participants