JsHint: Driven by NodeJs

[am11]

We are planning on rewriting JsHint compiler driven by NodeJs. As discussed with @madskristensen, we would have a standard .jshintrc (a global settings) file along with WE2013-settings.xml in User Profile folder.

Momentarily, In our JsHintRunner class, we have at least three ways to ignore js files, to skip them when seeding files to JsHintCompiler:

• [global scope] Hardcoded js files to ignore (_references.js etc.).
• [global scope] Ignore files added by user in Web Essentials settings.
• [local scope] .jshintignore file in the project folder or sub folders.

I am thinking about merging first two in one .jshintignore file and place it in the settings directory (along with .jshintrc file along with WE2013-settings.xml).

The catch:

This means we will no longer have anything related to JsHint in WE2013-settings.xml.

PROS:

• Standard configurations and settings files: .jshintrc and jshintignore.
• Easy to recover: We would have a folder settings-backup under Resources. So, if global file is missing under user profile, it will just File.Copy() without parsing and caching the contents.
• No additional overhead of parsing JSON, creating a final .jshintrc dynamically and yet engaging File I/O.

CONS (off the top of my head, there is one):

We won't be giving options under VS's Tools > Options > Web Essentials dialog. User would have to change settings in standard .jshintrc and .jshintignore files

Point to ponder:

.jshintrc is essentially a JSON file:

https://github.com/gruntjs/grunt-contrib-jshint/blob/master/.jshintrc
https://gist.github.com/haschek/2595796

Evidentially, these files can have single line comments. Unfortunately System.Web.Helpers.Json doesn't support parsing JSON with comments while NewtonSoft's JSON.NET only supports multi-line comments, and throws exception for single-line comments.(I guess the current implementation must also has this problem).

Therefore, provisioning of settings in VS's GUI options dialog is going to be problematic either way.

Have any suggestions, ideas? Please chip-in!

Thanks.

[SLaks]

BTW, we should move half of the files from the Margin directory to a new Compilers directory.

Mads, do you want to do that?
This should be done quickly to avoid merge conflicts in pull requests.

[madskristensen]

Let me make a 1.6 release now and then move it

[SLaks]

Note that JSON itself does not support comments; see the full syntax at http://www.json.org/

JSHint removes comments before parsing the file: https://github.com/jshint/jshint/blob/2.x/src/cli.js#L396

[madskristensen]

The JSON editor that we're building in VS will be able to handle comments even though some parsers can't handle comments.

[SLaks]

I think we should create a designer for jshintrc files with a property grid bound to a settings object parsed from JSON (like we already have in the JSHint settings files)

We can then allow users to right-click a project (or folder), Add New Item, JSHint Configuration, and get a nice UI to build it.
On that note, we should also add a template & icon for Markdown files in WE to avoid problems like #386 (comment). How is that done?

[madskristensen]

A .jshintrc item template is already in SideWaffle I believe. Same with .jshintignore.

[SLaks]

For ignores, I think we should get rid of the separate user-level ignore list in options & instead add a .jshintignore template.

We can add a ContentType for it with a classifier for comments & maybe some wildcard characters, and with IntelliSense for pathnames.
We should extract the directory IntelliSense code which is currently duplicated, with subtle variations, for CSS images & JS module paths & make it reusable for different scenarios. (I have plans for similar IntelliSense for a Markdown configuration file for assembly references)

[madskristensen]

I'll talk to the crew about a Markdown icon. It has to be registered on in Windows for it to take effect in Solution Explorer, but there might be a way that doesn't require Admin privileges.

[madskristensen]

Agreed, a great editor for .jshintignore would be awesome

[SLaks]

We can make a Windows file type without admin by writing to HKCU\Software\Classes\.md.
I assume that's easily doable in the pkgdef.

However, we also need a good icon.

[SLaks]

Speaking of which, we should add IntelliSense for Registry paths & values in .pkgdef, preferably by open-sourcing the extension you found.

[madskristensen]

Great then. I assume it just requires a .ico file, correct? I'll get on it

[SLaks]

Yes; preferably matching VS's icon style & with a 256px image for Explorer.

[am11]

Would be nice, if we have syntax highlighting (and intellisence if applicable) support for .reg, .bat, .com and .ps1 in Visual Studio. Some projects use these files and its very unexpected to see those opening as plain text in an IDE like Visual Studio. :)

@SLaks, thanks for the input. Nice idea about organizing compliers in one place. I was thinking about moving JsHint's variants to Margin. Also, would it make sense to change JsHintRunner to JsHintMargin : MarginBase and JsHinProjectRunner to JsHintProjectCompiler : ProjectCompilerBase?

@madskristensen, speaking of JSON, unlike Newtonsoft's JSON.NET, the current implementation of System.Web.Helpers.Json doesn't automatically convert Array / List types when parsing to Dynamic object. I reported the issue here https://connect.microsoft.com/VisualStudio/feedback/details/810981/bug-json-decode-modify-object-encode-ignores-array-value. Please consider this in the ongoing advancements of framework. 8-)

[SLaks]

None of this (including existing compilers) have anything to do with Margins; Mads will move all of those classes out of that folder soon.

After moving the files, we should extract the last bits of source map code from the margin classes to fully decouple them.

[SLaks]

Pull requests welcome.
https://aspnetwebstack.codeplex.com/SourceControl/latest#src/System.Web.Helpers/Json.cs

[SLaks]

Note that System.Web.Helpers.Json deserializes objects to dictionaries and therefore cannot preserve original property order.
It therefore cannot be used to edit JSON files.

JSON.Net does preserve property order, but AFAIK still doesn't preserve whitespace, making it less than perfect for this as well. I am not aware of any .Net JSON API that preserves whitespace tokens.

[SLaks]

In contrast, look at my code to preserve whitespace & comments for XML settings files in Rebracer; https://github.com/SLaks/Rebracer/blob/master/Rebracer/Utilities/XmlMerger.cs and https://github.com/SLaks/Rebracer/blob/master/Rebracer.Tests/UtilitiesTets/XmlMergerTests.cs#L196

[madskristensen]

@am11 VS uses JSON.NET internally. It's the best .NET implementation of a JSON parser and we should use it whenever possible instead of other serializers/parsers.

[am11]

From Bing images: http://www.bing.com/images/search?&q=markdown+icon

This one looks very VS'ish:

At the same time, I believe there are better options than this. B-)

[SLaks]

No; the lines are too thick & the angle doesn't belong.

[madskristensen]

Providing colorization, Intellisense, validation etc. for non-web file types such as .ps1, .reg and .bat lies outside what Web Essentials should cover. There are already extensions for .ps1 and .pkgdef available in the VS Gallery.

[madskristensen]

We can use whatever icon for POC purposes while we find a better icon. The point here is to have an icon for .md, and secondarily that it looks sweet

[am11]

@madskristensen, so would it be a modified (better) version of JSON.NET as there are some issues with JSON.NET (preserving whitespaces which @SLaks mentioned and handling single-line comments)?

[am11]

@madskristensen, please don't forget aboutthe IcedCoffeeScript icon (#360):

in contrast with:

[DerAlbertCom]

@am11 with the change of the vsix manifest this http://visualstudiogallery.msdn.microsoft.com/fd129629-a1a1-417c-ac80-c9ac7a67b968 runs in VS2013, and offers allot of syntax highlighting including .json

[madskristensen]

I think we should get the icon for .iced in VS itself since the CoffeeScript editor already colorizes IcedCoffeeScript constructs. I'll create a bug for it when I get back after vacation. I'll have them add the same coffee cup icon go .iced files.

[madskristensen]

@am11 For issues found with JSON.NET, we should contribute to that project to have them fixed instead of doing all the workarounds in WE.

[am11]

@SLaks, would it also differentiate between tabs (\t) and whitespace?

@madskristensen, that makes sense too! Btw, the iced coffee glass with straw one was taken from their official website: http://maxtaco.github.io/coffee-script/

[SLaks]

I've been very disappointed with TextHighlighterExtension.
It seems to use its own highlighting system instead of Classifiers; it's slow & non-standard.

[am11]

@madskristensen, YES! both sass and scss 👍

[SLaks]

@am11 We also need unit tests for JSHint compilation.

[am11]

Yes and now for SASS too. :)

[madskristensen]

@am11 @SLaks With the addition of a global .jshintrc, should we remove all the WE settings for JSHint? Also, when copying the WE-Settings.xml into the solution, should we also copy in the global .jshintrc? Will a .jshintrc file even be respected if it sits in the solution folder?

[madskristensen]

I just confirmed that .jshintrc is applied if placed on the solution level (one level above project in the file system). So should we just move the .jshintrc file over when users invoke Create solution settings and add it to Solution Items just like we do for WE-Settings.xml? What is the expected behavior here?

SideWaffle has both a .jshintrc and .jshintignore item templates, so we could just leave this step up to the user. The problem is that we have to remove the old JSHint settings from the XML store, so discoverability is going to be a problem here.

[SLaks]

To clarify, JSHint looks for the closest .jshintrc file in the parent directories of each file being linted.
We duplicate this logic here.

This happens regardless of solution structure; in particular, a solution-level .jshintrc won't work for projects that are physically located outside the solution directory.
To preserve compatibility with other JSHint tools (command-line, Sublime, etc), we shouldn't have our own custom settings location.
However, we still need to duplicate their settings locator code so that we can provide our user-global settings file if no other settings file exists.
If we get rid of the user-global settings file, we can get rid of all of that code & use JSHint's built-in logic only.

[am11]

OAN, should we merge the hardcoded list of ignore files and user-defined (persistent) files in a global .jsignore?

[madskristensen]

@am11 I think that would be cool

[madskristensen]

@SLaks That makes sense. The best thing is if WE just uses the regular JSHint behavior instead of supplying its own. Do you want to create an issue for that with a description of what needs to be done?

Then I think we can close this issue

[am11]

@SLaks, can we search .jshintrc in Solution Items separately?

If not found in ancestry till project root
    fall back to search in solution items

if not found in solution items
    fall back to global location

if not found at global location
    copy global from backup directory and use it

[am11]

@madskristensen, close it after the .jshintignore may be? :)

[am11]

Yup I guess its done as we have a separate issue for it.

[am11]

Do you think we should fix FindLocalSettings() to find in Solution Items the aforementioned way?

[madskristensen]

@am11 I think that makes sense. What does @SLaks think?

[SLaks]

@am11 As I said before, that isn't a good idea.
Other than (maybe) user-level defaults, we should not have any settings locator logic different from stock JSHint.

[madskristensen]

So how do we provide the default settings? Are they already set in JSHint and so there is no need for a global .jshintrc and .jshintignore?

[SLaks]

Yes; JSHint works fine without any config files.

[am11]

@SLaks, yes! That really makes sense.

So should we also get rid off the hardcoded files to ignore logic too?

[SLaks]

We can't simply get rid of that entirely, since JSHint won't skip those files by default.
We can move them to a WE-global .jshintignore, but we would then still need to at least locate .jshintignore ourselves. (note that we run ignore logic earlier in the pipeline)

I think it would make most sense to keep the hard-coded ignore list, but use default .jshintignore and .jshintrc files for everything else, and remove all config file code from WE.
This would let us remove Minimatch.Net & the two file locators.

Note that if we want to support a user-global .jshintrc, we need to keep all of our .jshintrc locator code (the code currently in JsHintCompiler, since the jshint CLI has no way to specify a default fallback location.
We could hack that by setting the HOME env variable in the jshint process to the location of the WE user-global .jshintrc file, or (preferably) simply by storing the file in the user's profile directory.
See
https://github.com/jshint/jshint/blob/2.x/src/cli.js#L469
and
https://github.com/jshint/jshint/blob/2.x/src/cli.js#L93-L94

[madskristensen]

Ok, so if I understand you correctly, the tasks are:

1. Keep the current ignore logic
2. Remove the JSHint rules options from the WE settings
3. Remove Minimatch.Net and the two file locators
4. Store the global .jshintrc in the user's profile directory

How does that sound?

[SLaks]

Perfect.

So the JSHint tab in WE Options would then edit either the user-profile or solution-level .jshintrc JSON directly.

We can use https://github.com/SLaks/ConfOxide, which is almost finished, to easily databind to the JSON file & preserve property order (but unfortunately not formatting).
Once that's finished, I intend to completely overhaul every bit of settings code, probably reducing that code size by 70%.

[madskristensen]

I'm not sure we can edit the .jshintrc from the WE Options since you can have multiple .jshintrc files in your project - one for each folder potentially - and not just a project root level .jshintrc.

If this is true, then the 1-4 tasks are correct, right?

[SLaks]

Yes.

We should then add a designer for .jshintrc files, and a menu item to edit the user-global .jshintrc in that editor.

[madskristensen]

I don't think we need a designer for .jshintrc since a JSON editor is coming in the same update to VS as the SASS editor is. It might even have schema driven Intellisense and validation, so we can give full Intellisense for .jshintrc.

I agree with adding a gesture to editing the user-global .jshintrc should be there. With this in mind, these are the tasks:

1. Keep the current ignore logic
2. Remove the JSHint rules options from the WE settings
3. Remove Minimatch.Net and the two file locators
4. Store the global .jshintrc in the user's profile directory
5. Add button to Web Essentials menu for opening the user-global .jshintrc file in the editor.

I'll close this issue and open #441 with all these tasks since they are so inter-connected.

[SLaks]

For .jshintrc files in particular, I think a designer would be more useful that a pure JSON editor.
Most options are booleans; a designer would let you easily scroll through a list of options & double-click to toggle them, whereas an editor, even with full schema IntelliSense, would make discoverability much worse and would make it more annoying to toggle.

[madskristensen]

I agree, but I don't think it is super important. It's more of a nice-to-have. Also, some users might be really annoyed by it

[SLaks]

True