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

Fix #6661. Configurable multiple linters. #7362

Merged
merged 22 commits into from
Dec 11, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6c017c4
Fix #6661. Configurable multiple linters.
busykai Mar 28, 2014
87ec908
Add testcase for non-existance provider.
busykai Apr 2, 2014
be33f39
Add test case for when prefer is not specified.
busykai Apr 2, 2014
a6d4601
Merge remote-tracking branch 'upstream/master' into kai/fix-6661-mult…
busykai Sep 25, 2014
19bdc3b
Merge remote-tracking branch 'upstream/kai/fix-6558-language-layer' i…
busykai Sep 25, 2014
f1e4c90
Merge remote-tracking branch 'upstream/kai/fix-6558-language-layer' i…
busykai Sep 26, 2014
87d59dd
Merge remote-tracking branch 'upstream/kai/fix-6558-language-layer' i…
busykai Sep 26, 2014
f43a3e4
Use preferences with context.
busykai Sep 26, 2014
2eb9f5d
Fix tests.
busykai Sep 26, 2014
5d0dafe
Do not unregister JSLint on another JS provider registration.
busykai Sep 26, 2014
5af3533
Configure Brackets to use JSLint and JSHint.
busykai Sep 26, 2014
8ae613a
Merge remote-tracking branch 'upstream/master' into kai/fix-6661-mult…
busykai Dec 3, 2014
720d1fb
Merge remote-tracking branch 'upstream/master' into kai/fix-6661-mult…
busykai Dec 3, 2014
49c8dbe
Fix JSON syntax error.
busykai Dec 3, 2014
5fcb3b9
Improve readability of var declarations.
busykai Dec 4, 2014
0b003f5
Nicer reduce.
busykai Dec 4, 2014
47399b6
Switch to array for 'prefer' pref.
busykai Dec 4, 2014
4232b7e
Allow specifying both string and array in `prefer`
busykai Dec 4, 2014
6389919
Fix testing left-over.
busykai Dec 4, 2014
d005def
Use variables for pref names.
busykai Dec 4, 2014
509417c
Switch to native reduce.
busykai Dec 4, 2014
fb0183f
Address review comments.
busykai Dec 9, 2014
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
8 changes: 7 additions & 1 deletion .brackets.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@
"es5": true
},
"defaultExtension": "js",
"language": {
"javascript": {
"linting.prefer": ["JSLint", "JSHint"],
"linting.usePreferredOnly": true
}
},
"path": {
"src/thirdparty/CodeMirror2/**/*.js": {
"spaceUnits": 2,
Expand All @@ -20,4 +26,4 @@
},
"spaceUnits": 4,
"useTabChar": false
}
}
59 changes: 43 additions & 16 deletions src/language/CodeInspection.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ define(function (require, exports, module) {
/**
* Constants for the preferences defined in this file.
*/
var PREF_ENABLED = "enabled",
PREF_COLLAPSED = "collapsed",
PREF_ASYNC_TIMEOUT = "asyncTimeout";
var PREF_ENABLED = "enabled",
PREF_COLLAPSED = "collapsed",
PREF_ASYNC_TIMEOUT = "asyncTimeout",
PREF_PREFER_PROVIDERS = "prefer",
PREF_PREFERRED_ONLY = "usePreferredOnly";

var prefs = PreferencesManager.getExtensionPrefs("linting");

Expand Down Expand Up @@ -158,11 +160,42 @@ define(function (require, exports, module) {
* Decision is made depending on the file extension.
*
* @param {!string} filePath
* @return ?{Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}>} provider
* @return {Array.<{name:string, scanFileAsync:?function(string, string):!{$.Promise}, scanFile:?function(string, string):?{errors:!Array, aborted:boolean}}>}
*/
function getProvidersForPath(filePath) {
var providers = _providers[LanguageManager.getLanguageForPath(filePath).getId()];
return (providers && providers.slice(0)) || [];
var language = LanguageManager.getLanguageForPath(filePath).getId(),
context = PreferencesManager._buildContext(filePath, language),
installedProviders = _providers[language],
preferredProviders,

prefPreferredProviderNames = prefs.get(PREF_PREFER_PROVIDERS, context),
prefPreferredOnly = prefs.get(PREF_PREFERRED_ONLY, context),

providers;

// ensure there is an instance and that a copy is returned, always
installedProviders = (installedProviders && installedProviders.slice(0)) || [];

if (prefPreferredProviderNames && prefPreferredProviderNames.length) {
if (typeof prefPreferredProviderNames === "string") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it nicer to use !Array.isArray(prefPreferredProviderNames)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typeof is faster. otherwise, there's no difference.

prefPreferredProviderNames = [prefPreferredProviderNames];
}
preferredProviders = prefPreferredProviderNames.reduce(function (result, key) {
var provider = _.find(installedProviders, {name: key});
if (provider) {
result.push(provider);
}
return result;
}, []);
if (prefPreferredOnly) {
providers = preferredProviders;
} else {
providers = _.union(preferredProviders, installedProviders);
}
} else {
providers = installedProviders;
}
return providers;
}

/**
Expand All @@ -184,7 +217,7 @@ define(function (require, exports, module) {
var response = new $.Deferred(),
results = [];

providerList = (providerList || getProvidersForPath(file.fullPath)) || [];
providerList = providerList || getProvidersForPath(file.fullPath);

if (!providerList.length) {
response.resolve(null);
Expand Down Expand Up @@ -458,14 +491,6 @@ define(function (require, exports, module) {
_providers[languageId] = [];
}

if (languageId === "javascript") {
// This is a special case to enable extension provider to replace the JSLint provider
// in favor of their own implementation
_.remove(_providers[languageId], function (registeredProvider) {
return registeredProvider.name === "JSLint";
});
}

_providers[languageId].push(provider);

run(); // in case a file of this type is open currently
Expand Down Expand Up @@ -580,7 +605,7 @@ define(function (require, exports, module) {
});

prefs.definePreference(PREF_ASYNC_TIMEOUT, "number", 10000);

// Initialize items dependent on HTML DOM
AppInit.htmlReady(function () {
// Create bottom panel to list error details
Expand Down Expand Up @@ -644,6 +669,8 @@ define(function (require, exports, module) {
// Testing
exports._unregisterAll = _unregisterAll;
exports._PREF_ASYNC_TIMEOUT = PREF_ASYNC_TIMEOUT;
exports._PREF_PREFER_PROVIDERS = PREF_PREFER_PROVIDERS;
exports._PREF_PREFERRED_ONLY = PREF_PREFERRED_ONLY;

// Public API
exports.register = register;
Expand Down
75 changes: 59 additions & 16 deletions test/spec/CodeInspection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ define(function (require, exports, module) {
var SpecRunnerUtils = require("spec/SpecRunnerUtils"),
FileSystem = require("filesystem/FileSystem"),
StringUtils = require("utils/StringUtils"),
Strings = require("strings");
Strings = require("strings"),
_ = require("thirdparty/lodash");

describe("Code Inspection", function () {
this.category = "integration";
Expand All @@ -42,8 +43,9 @@ define(function (require, exports, module) {
CodeInspection,
CommandManager,
Commands = require("command/Commands"),
DocumentManager,
EditorManager,
DocumentManager,
PreferencesManager,
prefs;

var toggleJSLintResults = function (visible) {
Expand Down Expand Up @@ -115,6 +117,7 @@ define(function (require, exports, module) {
EditorManager = brackets.test.EditorManager;
prefs = brackets.test.PreferencesManager.getExtensionPrefs("linting");
CodeInspection = brackets.test.CodeInspection;
PreferencesManager = brackets.test.PreferencesManager;
CodeInspection.toggleEnabled(true);
});
});
Expand Down Expand Up @@ -300,6 +303,60 @@ define(function (require, exports, module) {
});
});

it("should use preferences for providers lookup", function () {
var pm = PreferencesManager.getExtensionPrefs("linting"),
codeInspector1 = createCodeInspector("html1", failLintResult),
codeInspector2 = createCodeInspector("html2", successfulLintResult),
codeInspector3 = createCodeInspector("html3", successfulLintResult),
codeInspector4 = createCodeInspector("html4", successfulLintResult),
codeInspector5 = createCodeInspector("html5", failLintResult);

CodeInspection.register("html", codeInspector1);
CodeInspection.register("html", codeInspector2);
CodeInspection.register("html", codeInspector3);
CodeInspection.register("html", codeInspector4);
CodeInspection.register("html", codeInspector5);

function setAtLocation(name, value) {
pm.set(name, value, {location: {layer: "language", layerID: "html", scope: "user"}});
}

runs(function () {
var providers;

setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html3", "html4"]);
providers = CodeInspection.getProvidersForPath("my/index.html");
expect(providers).toNotBe(null);
expect(_.pluck(providers, "name")).toEqual(["html3", "html4", "html1", "html2", "html5"]);

setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html5", "html6"]);
providers = CodeInspection.getProvidersForPath("index.html");
expect(providers).toNotBe(null);
expect(_.pluck(providers, "name")).toEqual(["html5", "html1", "html2", "html3", "html4"]);

setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html19", "html100"]);
providers = CodeInspection.getProvidersForPath("index.html");
expect(providers).toNotBe(null);
expect(_.pluck(providers, "name")).toEqual(["html1", "html2", "html3", "html4", "html5"]);

setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, true);
providers = CodeInspection.getProvidersForPath("test.html");
expect(providers).toEqual([]);

setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, ["html2", "html1"]);
setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, true);
providers = CodeInspection.getProvidersForPath("c:/temp/another.html");
expect(providers).toNotBe(null);
expect(_.pluck(providers, "name")).toEqual(["html2", "html1"]);

setAtLocation(CodeInspection._PREF_PREFER_PROVIDERS, undefined);
setAtLocation(CodeInspection._PREF_PREFERRED_ONLY, undefined);
providers = CodeInspection.getProvidersForPath("index.html");
expect(providers).toNotBe(null);
expect(_.pluck(providers, "name")).toEqual(["html1", "html2", "html3", "html4", "html5"]);
});
});

it("should run asynchoronous implementation when both available in the provider", function () {
var provider = createAsyncCodeInspector("javascript async linter with sync impl", failLintResult(), 200, true);
CodeInspection.register("javascript", provider);
Expand Down Expand Up @@ -1071,20 +1128,6 @@ define(function (require, exports, module) {
CodeInspection._unregisterAll();
});

it("should unregister JSLint linter if a new javascript linter is registered", function () {
var codeInspector1 = createCodeInspector("JSLint", successfulLintResult());
CodeInspection.register("javascript", codeInspector1);
var codeInspector2 = createCodeInspector("javascript inspector", successfulLintResult());
CodeInspection.register("javascript", codeInspector2, true);

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

runs(function () {
expect(codeInspector1.scanFile).not.toHaveBeenCalled();
expect(codeInspector2.scanFile).toHaveBeenCalled();
});
});

it("should call inspector 1 and inspector 2", function () {
var codeInspector1 = createCodeInspector("javascript inspector 1", successfulLintResult());
CodeInspection.register("javascript", codeInspector1);
Expand Down