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

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

Closed
ingorichter wants to merge 2 commits into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc5cc6b
- modified the problems panel to support multiple code inspector
ingorichter Sep 17, 2013
09bc17f
Fixed the issues mentioned in the code review.
ingorichter Sep 23, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/extensions/default/JSLint/unittests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ define(function (require, exports, module) {
$,
brackets,
CodeInspection,
EditorManager;

EditorManager,
previousState;

var toggleJSLintResults = function (visible) {
$("#status-inspection").triggerHandler("click");
expect($("#problems-panel").is(":visible")).toBe(visible);
Expand All @@ -50,9 +51,12 @@ define(function (require, exports, module) {
// Load module instances from brackets.test
$ = testWindow.$;
brackets = testWindow.brackets;
EditorManager = testWindow.brackets.test.EditorManager;
CodeInspection = testWindow.brackets.test.CodeInspection;
EditorManager = brackets.test.EditorManager;
CodeInspection = brackets.test.CodeInspection;
Copy link
Member

@peterflynn peterflynn Oct 18, 2013

Choose a reason for hiding this comment

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

I think the testWindow. prefix is still needed -- otherwise you're running the test in the spec-runner window instead.

CodeInspection.toggleEnabled(true);
// enable JSLint and preserve the previous state before we enable it for testing
previousState = CodeInspection._getProviderState({name: "JSLint"});
CodeInspection._setProviderState({name: "JSLint"}, true);
});
});

Expand All @@ -67,17 +71,16 @@ define(function (require, exports, module) {
brackets = null;
EditorManager = null;
SpecRunnerUtils.closeTestWindow();

// revert to previous state
Copy link
Member

@peterflynn peterflynn Oct 18, 2013

Choose a reason for hiding this comment

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

It seems valuable to still make sure the correct provider was invoked, too. I guess a true "unit" test would use one or more mock providers instead of the full-blown JSLint provider, though...

CodeInspection._setProviderState({name: 'JSLint'}, previousState);
Copy link
Member

@peterflynn peterflynn Oct 18, 2013

Choose a reason for hiding this comment

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

Shouldn't be needed if using testWindow for the test

});

it("should run JSLint linter when a JavaScript document opens", function () {
runs(function () {
spyOn(testWindow, "JSLINT").andCallThrough();
});

waitsForDone(SpecRunnerUtils.openProjectFiles(["errors.js"]), "open test file");

runs(function () {
expect(testWindow.JSLINT).toHaveBeenCalled();
expect($("#problems-panel").is(":visible")).toBe(true);
});
});

Expand All @@ -98,6 +101,5 @@ define(function (require, exports, module) {
toggleJSLintResults(false);
});
});

});
});
9 changes: 7 additions & 2 deletions src/htmlContent/problems-panel-table.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

@peterflynn peterflynn Oct 18, 2013

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

@peterflynn peterflynn Oct 18, 2013

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)

{{#results}}
<tr>
<td class="line-number" data-character="{{pos.ch}}">{{friendlyLine}}</td>
<td>{{message}}</td>
<td>{{codeSnippet}}</td>
<td class="line-text">{{message}}</td>
<td class="line-snippet">{{codeSnippet}}</td>
</tr>
{{/results}}
{{/reportList}}
</tbody>
</table>
2 changes: 1 addition & 1 deletion src/htmlContent/problems-panel.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<div id="problems-panel" class="bottom-panel vert-resizable top-resizer">
<div class="toolbar simple-toolbar-layout">
<div class="title"><!-- filled in programmatically with active linter's name --></div>
<div class="title"><!-- filled in programmatically --></div>
<a href="#" class="close">&times;</a>
</div>
<div class="table-container resizable-content"></div>
Expand Down
Loading