Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make JavaScript ignore rule better #636

Merged
merged 1 commit into from
Feb 8, 2014

Conversation

ChrisTorng
Copy link
Contributor

Includes JSHint, JSCS, default JSCS excludeFiles, with additional
improvement with test project VSHost typing faster.

I know this must be the top priority work for you all. Must be someone working on it. I just try to contribute.

For JsHintReporter.cs, extract NotJsOrIsMinifiedOrNotExists() from ShouldIgonre(), to be use in MenuItems/JsCodeStyle.cs and JsHint.cs. I think minified js should never been JSHint/JSCS. For other default ignored js, they will still be ignore at build. But if user select them explicitly and choose to run JSHint/JSCS, we should not stop it.

For LintFileInvoker.cs, I try to stop it run on non project items. But in my debug test, I don't know why it never hit. Still commit it.

Set .jcsc.json excludeFiles to ["**/*.min.js", "**/*.debug.js", "**/*.intellisense.js", "**/*-vsdoc.js"].

For VSHost.cs, original TypeChar will init/cleanup with every single char, I change it into init/cleanup once for a whole string, improve efficiency. And according to http://www.pinvoke.net/default.aspx/Structures/VARIANTARG.html, I change 32 into 16.
This one is not related to others, I don't know whether I should commit/sync/PR this small thing alone, or just put it with other changes like this time.

Includes JSHint, JSCS, default JSCS excludeFiles, with additional
improvement with test project VSHost typing faster.
@ChrisTorng
Copy link
Contributor Author

For non-project files issue, I agree with you.

I only apply it in LintFileInvoker.RunOnAllFilesInProjectAsync().
As my inspection, RunOnAllFilesInProjectAsync() have 3 references, all from EditorExtensionsPackage.BuildEvents_OnBuildDone(), it should be called only after build.
And only run after build succeed and while WESettings.Instance.JavaScript/TypeScript.LintOnBuild == true.

For a "Run on build" request, I think what user expected is build on project referenced items, just like VS will build on all cs that been referenced, never touch other unreferenced cs inside project folder.

And I didn't stop it while open from "File - Open - File", or right click (with "Show All Files" on) to run.

Oops! While I'm checking, I see JsHintReporter.ShouldIgnore() have check for non-project items. That stops right click menu from working. But that method is shared with other callers. I cannot figure whether the checking should be removed, or it still need to be apply with some other cases.

madskristensen added a commit that referenced this pull request Feb 8, 2014
Make JavaScript ignore rule better
@madskristensen madskristensen merged commit 991674f into madskristensen:master Feb 8, 2014
@SLaks
Copy link
Collaborator

SLaks commented Feb 9, 2014

Thanks!
BTW, for the future, please put separate fixes like that in a separate commit within the same PR (for a more readable commit history)

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

Successfully merging this pull request may close these issues.

3 participants