JSLint: set `browser: true` by default #3310

alfredxing · 2013-04-01T19:58:56Z

Set the option /* jslint browser: true */ by default,
because Brackets is for "Coding the Web".

Addresses issue #3269

Set the option `/* jslint browser: true */` by default, because Brackets is for "Coding the Web".

peterflynn · 2013-04-01T21:04:14Z

Copying my comment from the bug:

We should be careful of the potential confusion though: this will mean the results of running JSLint within Brackets differ from the results of running JSLint in a build script, Grunt task, jslint.com portal, etc.

alfredxing · 2013-04-01T21:23:15Z

@peterflynn True... Should we have it as an option that users can enable?

jdiehl · 2013-04-02T06:01:01Z

IMO, the best solution would be a configuration dialog, where users can enter their jslint settings by copying the generated directive from http://jslint.com

TomMalbran · 2013-04-02T06:15:11Z

Yes, if we can apply it as a global preference, then that would be pretty easy, if it ends up being a project or file preference then it will not be, since project/file preferences aren't ready yet.

jdiehl · 2013-04-02T06:34:52Z

I agree, a global preference would be the way to start (until there is a proper Brackets way to store global preferences you can use window.localStorage), which can later be overridden by project preferences. File preferences probably do not make too much sense as JSLint has a built-in way to store them.

TomMalbran · 2013-04-02T06:43:48Z

The PreferenceStorage is used to save global preferences, which actually uses the local storage. It could also be used for files/projects, but there isn't a way to automatically delete them for deleted/unused project/files or show all of them and let the user delete them if he wants to.

alfredxing · 2013-04-02T15:38:29Z

IMO, the best solution would be a configuration dialog, where users can enter their jslint settings by copying the generated directive from http://jslint.com

@jdiehl I'm not so sure about that one, since the JavaScript function takes in an object of options like {browser: true}. If you wanted something like that you would probably have to append the text before every file passing through JSLint? It's definitely possible but the configuration dialog makes it a bit harder.

I was thinking a dialog with checkboxes like those on the website; maybe that's a bit easier for the user as well? It would be easier to store the options as well (as JSON).

jdiehl · 2013-04-02T16:24:12Z

You could easily parse the string from the website and create the JSON object for the function call, so that should not be a problem.

Interaction-wise, I like utilizing existing resources if possible (the JSLint website and its form) and giving users powerful access to the low-level features (being able to copy&paste settings between different Brackets installations, etc.) without imposing a difficult interaction. The JSLint form could be easily linked or even embedded in the configuration dialog (we are working with web technology so everything is possible!). Additionally, there is no need to update the dialog if the JSLint options change. But, yes, a checkbox dialog would also work very well.

njx · 2013-04-02T17:19:45Z

@adrocknaphobia - @peterflynn pointed out that if we make this change, it will make our in-product JSLint inconsistent with what users would get from running it standalone. Would we still want this change in that case?

peterflynn · 2013-04-02T17:43:10Z

Another option would be to implement support for the ".jslintrc" config file described here: https://groups.google.com/d/msg/brackets-dev/PGeYH5KTQNI/nd8B3xPmrhUJ. That wouldn't let you have "browser: true" as a default for all projects you open, but it would let you easily make it the default for a given project without having to add a JSLint settings block in every individual .js file.

This would also be more flexible -- IMHO, given the strictness of JSLint, you will almost certainly want to have other settings set to non-default values too. If we took the prefs approach, unless we added UI for every JSLint setting, you'd still most likely need to as a JSLint settings block in every .js file to override those other settings. With a global project file, we could support project-global defaults for all settings with less effort.

jdiehl · 2013-04-02T18:00:03Z

I really like the .jslintrc approach and the more editors / jslint tools support this the better it will become.

The process could be as following:

Load project options from [PROJECT_ROOT]/.jslintrc
If no project options exist, load global options from ~/.jslintrc
Parse the options (strip /* and */ and run JSON.parse) and use the resulting object to configure the jslint parser

The configuration dialog (if added) then simply allows users to edit these files (PreferenceStorage is not used) with a simple editor (textarea) or checkboxes.

To inform users about the difference between their jslint run in Brackets and an online run, the tool could run jslint a second time without the project/global options in the background or on demand (toggle button, tooltip, etc.).

Add an option in a dialog to allow users to set a global JSLint directive. The dialog trigger is located in the *File > Set JSLint Global Directive...* menu.

Roll back ae5ecc4

Add an option in a dialog to allow users to set a global JSLint directive. The dialog trigger is located in the *File > Set JSLint Global Directive...* menu.

alfredxing · 2013-04-02T20:51:01Z

Sorry about the first commit - the formatting wasn't quite right.

jdiehl · 2013-04-02T20:59:52Z

src/extensions/default/JSLint/main.js

+        if (response === "ok" && typeof prefs !== "undefined") {
+            localStorage["jslint-directive"] = prefs;
+            var _obj = {};
+            var _props = prefs.replace("/*jslint ", "").replace(" */", "").split(', ');


This should use regular expressions to make the matching more reliable:

/^\s*\/\*\s*jslint\s*/ and /\/\s*\*\/\s*$/

I'll fix that, thanks!

jdiehl · 2013-04-02T21:15:37Z

I think this is a good start, but we should wait for the core team to decide on the inconsistency issue before putting any more effort into this.

alfredxing · 2013-04-02T21:32:11Z

@jdiehl I've implemented all of your suggestions locally (and everything's running great), but I won't push until that decision is made. Thanks!

TomMalbran · 2013-04-02T22:18:55Z

src/extensions/default/JSLint/main.js

+     */
+    function _savePrefs(prefs, response) {
+        if (response === "ok" && typeof prefs !== "undefined") {
+            localStorage["jslint-directive"] = prefs;


Use the preference storage here: _prefs.setValue("directive", prefs) and _prefs.getValue("directive") to retrieve them. You can set a default value changing to defaultPrefs = { enabled: true, directive: "" }.

alfredxing · 2013-04-04T23:21:19Z

@peterflynn @njx What do you guys think?

Add dialog to JSLint allowing users to paste a default directive.

isonno · 2013-04-19T00:45:03Z

I find it annoying that I can't turn JSLint off, and there doesn't seem to be a simple way to close the JSLint pane when it pops up (no close box?).

njx · 2013-04-19T00:47:41Z

@isonno -- you can turn it off from View > Enable JSLint, though I agree that it's annoying that you can't just close the panel. We plan to improve the UI around this when we make linting an "official" feature (it's currently an "unofficial" feature that happens to be in your face by default :))

njx · 2013-04-19T00:48:09Z

Marking PM since I won't be at the pull request review meeting tomorrow--need @adrocknaphobia's input on this.

peterflynn · 2013-04-19T21:26:07Z

I've added a link to this from the 'Linting Configuration' user story.

@alfredxing We may want nicer UI-driven configuration eventually, but IMHO this is still a useful step forward that would at least be nice as a stopgap. We're a little busy in the current sprint though, so we may not be able to merge this for another couple weeks. Bear with us! :-)

peterflynn · 2013-04-19T21:30:56Z

Oops, actually sorry I misread the thread. It looks like this doesn't implement my and @jdiehl's suggestions about .jslintrc... so I am actually less positive on this than what I said above. @alfredxing would you be interested in taking a stab at .jslintrc support? (If so best to make a new branch so it's separate from this UI work).

alfredxing · 2013-04-19T21:52:15Z

@peterflynn Sure. I have exams right now (they end Monday), but I'll get on to that right afterwards.

alfredxing · 2013-04-23T06:27:31Z

I tried using NativeFileSystem.resolveNativeFileSystemPath to get the file entry from the .jslintrc path, but it doesn't seem to think that .jslintrc is a file... Is there any other way to do that?

njx · 2013-05-10T04:41:58Z

To @adrocknaphobia due to [PM] tag. I'm not totally clear on what the status of this is from the comment thread--might want to ping @peterflynn to get his take. Once resolved, please ping the team to let us know that we should assign this.

Related to issues adobe#3269, adobe#4210. Based on the discussion and suggestions in adobe#3310. If a project has a .jslint.json in its root, it gets loaded and passed to each JSLint run within the project. The configuration file gets reloaded each time it is saved or refreshed.

njx · 2014-03-08T19:44:00Z

@dangoor @peterflynn - do you think this is obsolete now that we have preferences for it? It's not exactly .jslintrc support but it allows you to configure jslint from the prefs file.

TomMalbran · 2014-03-08T19:50:29Z

It also should be pretty easy to write an extension to support .jslintrc or other jslint files, by just grabbing the prefs from those files and apply them to the brackets preferences.

dangoor · 2014-03-10T13:59:11Z

@TomMalbran That seems like a good thing to incorporate into the main JSLint extension. If it sees a .jslintrc, it should prefer those settings over the ones in .brackets.json.

@njx looking at the original description of this issue, I think this one is done because we have done exactly what the issue is requesting.

njx · 2014-03-10T19:26:59Z

Got it. Closing.

JSLint: set browser:true by default

fad5468

Set the option `/* jslint browser: true */` by default, because Brackets is for "Coding the Web".

Alfred Xing and others added 3 commits April 2, 2013 13:32

Dialog for user to paste global JSLint directive

ae5ecc4

Add an option in a dialog to allow users to set a global JSLint directive. The dialog trigger is located in the *File > Set JSLint Global Directive...* menu.

Roll back commit

40a7164

Roll back ae5ecc4

Dialog for user to paste global JSLint directive

3addef5

Add an option in a dialog to allow users to set a global JSLint directive. The dialog trigger is located in the *File > Set JSLint Global Directive...* menu.

jdiehl reviewed Apr 2, 2013
View reviewed changes

TomMalbran reviewed Apr 2, 2013
View reviewed changes

Update JSLint with recommendations and dialog

9731131

Add dialog to JSLint allowing users to paste a default directive.

ghost assigned adrocknaphobia May 10, 2013

busykai mentioned this pull request Jul 5, 2013

[Arch] Add support for project-wide JSLint config. #4380

Closed

dangoor assigned dangoor and unassigned adrocknaphobia Mar 10, 2014

njx closed this Mar 10, 2014

core-ai-bot mentioned this pull request Aug 29, 2021

[CLOSED] JSLint: set browser: true by default brackets-archive/bracketsIssues#3112

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSLint: set `browser: true` by default #3310

JSLint: set `browser: true` by default #3310

alfredxing commented Apr 1, 2013

peterflynn commented Apr 1, 2013

alfredxing commented Apr 1, 2013

jdiehl commented Apr 2, 2013

TomMalbran commented Apr 2, 2013

jdiehl commented Apr 2, 2013

TomMalbran commented Apr 2, 2013

alfredxing commented Apr 2, 2013

jdiehl commented Apr 2, 2013

njx commented Apr 2, 2013

peterflynn commented Apr 2, 2013

jdiehl commented Apr 2, 2013

alfredxing commented Apr 2, 2013

jdiehl Apr 2, 2013

alfredxing Apr 2, 2013

jdiehl commented Apr 2, 2013

alfredxing commented Apr 2, 2013

TomMalbran Apr 2, 2013

alfredxing commented Apr 4, 2013

isonno commented Apr 19, 2013

njx commented Apr 19, 2013

njx commented Apr 19, 2013

peterflynn commented Apr 19, 2013

peterflynn commented Apr 19, 2013

alfredxing commented Apr 19, 2013

alfredxing commented Apr 23, 2013

njx commented May 10, 2013

njx commented Mar 8, 2014

TomMalbran commented Mar 8, 2014

dangoor commented Mar 10, 2014

njx commented Mar 10, 2014

JSLint: set browser: true by default #3310

JSLint: set browser: true by default #3310

Conversation

alfredxing commented Apr 1, 2013

peterflynn commented Apr 1, 2013

alfredxing commented Apr 1, 2013

jdiehl commented Apr 2, 2013

TomMalbran commented Apr 2, 2013

jdiehl commented Apr 2, 2013

TomMalbran commented Apr 2, 2013

alfredxing commented Apr 2, 2013

jdiehl commented Apr 2, 2013

njx commented Apr 2, 2013

peterflynn commented Apr 2, 2013

jdiehl commented Apr 2, 2013

alfredxing commented Apr 2, 2013

jdiehl Apr 2, 2013

Choose a reason for hiding this comment

alfredxing Apr 2, 2013

Choose a reason for hiding this comment

jdiehl commented Apr 2, 2013

alfredxing commented Apr 2, 2013

TomMalbran Apr 2, 2013

Choose a reason for hiding this comment

alfredxing commented Apr 4, 2013

isonno commented Apr 19, 2013

njx commented Apr 19, 2013

njx commented Apr 19, 2013

peterflynn commented Apr 19, 2013

peterflynn commented Apr 19, 2013

alfredxing commented Apr 19, 2013

alfredxing commented Apr 23, 2013

njx commented May 10, 2013

njx commented Mar 8, 2014

TomMalbran commented Mar 8, 2014

dangoor commented Mar 10, 2014

njx commented Mar 10, 2014

JSLint: set `browser: true` by default #3310

JSLint: set `browser: true` by default #3310