-
Notifications
You must be signed in to change notification settings - Fork 193
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
Support telling us what line and what file the error is commig from #240
Comments
This should be high priority since without telling you this information everything gets harder to fix the jshint errors. |
@paladox could you please provide the current and the expected output. Thanks! |
Currently when it lints the file and finds some errors it does this Actual outcome 19:13:40 Running "jshint:all" (jshint) task Expected outcome is so it looks a bit like this.Th19:30:47 Running "jscs:all" (jscs) task 19:31:11 Illegal trailing whitespace at ./resources/src/mediawiki/mediawiki.Uri.js : This is how grunt-jscs does it and the expected outcome for this grunt plugin. It shows you the line and file the error is comming form. |
So it tells you the file and line the error is in. But it dosent in grunt-contrib-jshint please could this be changed so it can because grunt-jscs does and grunt-jsonlint does. |
I am also seeing this problem. Previously, this was working fine - I could see what file each error was coming from. Now I don't see a filename above each error. Suppose I have 4 errors in 2 files out of a total of 15 files. It should say I haven't been able to tie this to a package version. Even on downgrading to |
It has the line the error is comming from but doesn't tell your the file. Also we should add --> to tell you which piece of the code is actually causing the error please. |
Yeah seems like a regression, probably a change in naming. Should be an easy pull request |
I tested with 0.11.3 and it dosent work. So this is supported just the code needs updating. |
@vladikoff Could you open the pull since you know how and where to fix it please. |
Yes it worked before but it seems to depend on which repo you test for example I am testing it on mediawiki/core which it wont show which file has the error but if I test it in mediawiki/extensions/CollapsibleVector it shows the error in which file. |
@vladikoff This is definitely a regression. Without the path to the file, the output is pretty much useless. This was working at least in grunt-contrib-jshint version 0.11.3. No longer works in 0.12.0. |
So here's the offending line: https://github.com/gruntjs/grunt-contrib-jshint/blob/master/tasks/lib/jshint.js#L95 If the ".bold" statement is removed, file paths start showing-up again using the default Grunt reporter. This is with the latest version (0.12.0). Is see the same line in 0.11.3 code, but strangely it works when I run the plugin from a tree I installed about 3 months ago, but when I force a new tree to install version 0.11.3, the paths don't show-up. AFAIKT, both 0.11.3 trees are using the same code and same dependencies, but it's hard to tell. Obviously, there's still a difference somewhere. Perhaps there's a subtle difference in the many dependencies that grunt 0.4.5 references since those are all using version ranges, so I'm likely using a slightly different grunt in my new tree I just installed vs my old tree? |
@stefcameron thanks for investigating, checking ... |
@stefcameron I just reinstalled the modules, I see the file name here if I remove a semicolon somewhere in the file:
|
could be related to jshint/jshint#2841 Different reporters? |
Yes it seemed to work for me on some repos but on others it didn't. Seems to be a bug then. |
@vladikoff Hmmm... I'm not intimately familiar with all the inner components involved. I tried uninstalling grunt-contrib-jshint, cleaning npm cache, reinstall grunt-contrib-jshint, but I still see the issue of missing file paths. The jshint issue you pointed to sounds very similar. Perhaps for now remove ".bold" until the root cause is found? I'm not sure where this property comes from, especially on a string. I guess something somewhere is mixing some formatting properties on the string object, but I don't know how it's all supposed to work. |
Using https://github.com/sindresorhus/jshint-stylish as the reporter seems to be a good workaround for now. |
Actually https://github.com/sindresorhus/jshint-stylish looks nice we should use that since it is clearly explained which line and file and is in style. But doesn't show the code that is causing it just the line and file. |
@vladikoff Running jshint from the command straight out of "node_modules\grunt-contrib-jshint\node_modules.bin\jshint" uses jshint's default console reporter which logs a simple string to console.log() here: https://github.com/jshint/jshint/blob/master/src/reporters/default.js#L31 It works great, and the file name is output. So I'm not convinced this issue is related to jshint/jshint#2841. I think this issue is specifically related to the ".bold" property access I pointed-out earlier in this thread. I can't figure-out where the JavaScript String class is getting augmented with such a property (which I presume is intended to produce bolded console output), but all I can conclude is that for whatever reason, the mixin isn't working (at least for some of us) and accessing My vote here is to simply remove ".bold"... |
Interestingly, when running this plugin from within SublimeText and having its output to go Sublime's console, the file paths come out as ...Yes! Now we're getting somewhere. Changing line https://github.com/gruntjs/grunt-contrib-jshint/blob/master/tasks/lib/jshint.js#L95 to At least that's a string with the path so |
@stefcameron But I think we should switch that out for chalk and avoid modifying the string prototype. |
Just updated and running into this issue. Without references to the file, jshint's usefulness drops considerably. It doesn't sound like there's a workaround at this point, is that correct? |
As mentioned above, using https://github.com/sindresorhus/jshint-stylish as the reporter is an easy immediate workaround to this. |
Please support telling us which line and which file the error is coming from instead of just showing code and saying the error is in
this number off files. Since if you have a lot of files you wont know which file is doing it.
grunt-jscs and grunt-jsonlint support this.
The text was updated successfully, but these errors were encountered: