search

cfjedimaster / brackets-jshint

Adds JSHint support to Brackets
MIT License
130 stars 41 forks source link

Brackets hangs during JSHint scan of large file #77

Open jbmonroe opened 9 years ago

jbmonroe commented 9 years ago

I have a JS file that's an array of arrays used for games that need a dictionary. It's 1362KB in size, and upon loading it in Brackets, JSLint embarks on an epic journey to try to lint it.

Well, it doesn't need linting--just editing. Brackets hangs up like a jilted girlfriend during the lint; the current web version of JSHint eventually struggles through the file, but it takes a very long time.

Not your fault, for sure, but still an issue. An option for the plug-in that disallowed linting any file larger than X KB would likely resolve this and still allow linting files of a more sane size. Any JS file larger than 100K probably needs to be broken down into smaller modules. Unfortunately for the dictionary, breaking it down into smaller files isn't really an option.

cfjedimaster commented 9 years ago

Unfortunately, the Linting API doesn't allow for that. My extension basically tells Brackets, "Hey, I lint js files", and Brackets handles passing the file to the extension when it works with one. In theory I could check the file size before I parse I suppose, but there is no good way to tell the user that the file will be ignored. I'm open to suggestions here.

jbmonroe commented 9 years ago

Hypothetically, if I'm the one who is setting the linting threshold, I'd be able to surmise a priori that the file wouldn't be linted. On the other hand, I can see some poor schlub sending in bug reports because there's no notification. If you checked the length of the incoming text and popped an error report versus line 1 would that do it? (I haven't looked at the linting API in depth so I'm just making stuff up as I go along.)

I suppose I could make a suggestion to both Crockford (because he really likes suggestions) and the JSHint folks to add a "don't lint this" to the config comment to get around it. It wouldn't do much for the on-line versions but it would go a long way for Brackets.

I realize that this particular case is a far outlier, and I don't relish the idea of coming up with a stupid/clever way to load the entire dictionary in parts. (Yeah, I know--RequireJS. I must confess to not entirely being a fan just yet, and since this is just an array of arrays, it doesn't fit the model.)

I'm still thinking.

cfjedimaster commented 9 years ago

You can disable linting for a specific file by using your main preferences - https://github.com/adobe/brackets/wiki/How-to-Use-Brackets#preferences

busykai commented 9 years ago

Well, a proper solution, it seems to me, that this linting could be offloaded to node (say, lint files bigger than x on node to avoid blocking the editor's thread) or probably in a worker. I'll do it if I ever find a free hour.

jbmonroe commented 9 years ago

Yeah, a worker could do it. I run Node, but does everyone?

busykai commented 9 years ago

Brackets has a node instance for certain purposes (e.g. extensions installation, live preview etc.). Every time you run Brackets, it starts a node.

cfjedimaster commented 9 years ago

Yeah, to be fair, this seems overkill for Node. I'd have to completely rewrite the extension versus, perhaps, just doing a line check before parsing. Honestly I'd use the Preferences like I suggested.

On Tue, May 12, 2015 at 8:31 AM, busykai notifications@github.com wrote:

Brackets has a node instance for certain purposes (e.g. extensions installation, live preview etc.). Every time you run Brackets, it starts a node.

— Reply to this email directly or view it on GitHub https://github.com/cfjedimaster/brackets-jshint/issues/77#issuecomment-101278835 .

Raymond Camden, Developer Advocate for MobileFirst at IBM

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: raymondcamden

jbmonroe commented 9 years ago

I think Preferences makes sense at this point. Despite it being the #1 problem in my life it's really a corner case.

On Tue, May 12, 2015 at 9:32 AM, Raymond Camden notifications@github.com wrote:

Yeah, to be fair, this seems overkill for Node. I'd have to completely rewrite the extension versus, perhaps, just doing a line check before parsing. Honestly I'd use the Preferences like I suggested.

On Tue, May 12, 2015 at 8:31 AM, busykai notifications@github.com wrote:

Brackets has a node instance for certain purposes (e.g. extensions installation, live preview etc.). Every time you run Brackets, it starts a node.

— Reply to this email directly or view it on GitHub < https://github.com/cfjedimaster/brackets-jshint/issues/77#issuecomment-101278835

.

Raymond Camden, Developer Advocate for MobileFirst at IBM

Email : raymondcamden@gmail.com Blog : www.raymondcamden.com Twitter: raymondcamden

— Reply to this email directly or view it on GitHub https://github.com/cfjedimaster/brackets-jshint/issues/77#issuecomment-101279203 .

busykai commented 9 years ago

@jbmonroe, this being your topmost problem, it's a wonderful life you're having. :)

@cfjedimaster, I agree with you pretty much on the ground that any file which takes any noticeable time to lint either 1) should not be linted; or 2) should be split into files of less size. and if it's 1) preferences are the right way.