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

Implement jscodehints.defaultExclusions preference. - #7739

Merged
JeffryBooher merged 3 commits into
masterfrom
dangoor/7262-ionic-blacklist
May 5, 2014
Merged

JeffryBooher merged 3 commits into
masterfrom
dangoor/7262-ionic-blacklist

Conversation

@dangoor

@dangoor dangoor commented May 3, 2014

Copy link
Copy Markdown
Contributor

We've had a number of people report problems with Ionic in their projects.
Tern appears to have an issue with the minified Ionic files. I had planned
to add default exclusions so that we could quickly solve pain like this
and this problem appeared to be a good candidate. The repro steps
reported here
are fixed with this change.
#7262

We've had a number of people report problems with Ionic in their projects.
Tern appears to have an issue with the minified Ionic files. I had planned
to add default exclusions so that we could quickly solve pain like this
and this problem appeared to be a good candidate. The repro steps
[reported here](#7262 (comment))
are fixed with this change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

brilliant!

@dangoor

dangoor commented May 5, 2014

Copy link
Copy Markdown
Contributor Author

I have changed the code to use globmatch, per @redmunds feedback. I tried "*.min.js" just to see how that would work out. I decided against it, because lodash.min.js actually produces fine hints and causes no trouble for Tern.

@JeffryBooher JeffryBooher self-assigned this May 5, 2014
@peterflynn

Copy link
Copy Markdown
Member

@dangoor Given the rising number of reports we've gotten specifically around Ionic, do you think this is worth cherry-picking into the EC release?

@JeffryBooher

Copy link
Copy Markdown
Contributor

Point of reference -- I was able to reproduce this eventually with the steps listed in the issue on Mac but the steps don't work on Windows. I presume because the cordova ios package isn't available on Windows for some reason.

I tested it on both mac and windows with the exclusions and it doesn't WSOD.

Code looks good. I presume we want to take this for Release 39?

@dangoor

dangoor commented May 5, 2014

Copy link
Copy Markdown
Contributor Author

@JeffryBooher yes, for 39

@peterflynn Good question. I don't think this change is risky and should be an easy merge, but it's also kind of late and Ionic is not quite like jQuery in terms of userbase... Given that we still need to do the EC release testing, it may be worth taking this – I think potential bugs would present themselves pretty quickly.

@JeffryBooher

Copy link
Copy Markdown
Contributor

Merging

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants