Code Monkey home page Code Monkey logo

grunt-contrib-jshint's People

Contributors

billwolckenlmco avatar cowboy avatar dependabot[bot] avatar giltron avatar gruber76 avatar jamesplease avatar jsr6720 avatar mgol avatar nickbabcock avatar nschonni avatar ofirdagan avatar paladox avatar pdehaan avatar robwierzbowski avatar scottnonnenberg avatar sebdeckers avatar shama avatar shinnn avatar sindresorhus avatar sjhewitt avatar sprzybylski avatar sudodoki avatar teppeis avatar thomasboyt avatar tiross avatar tonylukasavage avatar vladikoff avatar willfarrell avatar xhmikosr avatar xzyfer avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

grunt-contrib-jshint's Issues

Add support for .jshintignores

It would be great to have support for .jshintignores. Porting larger projects away from jshint cli to this plugin is sometimes not possible.

warnOnly option

Is there a way to avoid having the jshint task stopping my grunt task on failure.

Use case is when I start my dev server with grunt dev, I want to see jshint output in the console, but I don't want to prevent the dev server running if there are jshint issues. (however when I build the prod site with grunt build I want jshint issues to stop the build process.)

I've implemented this workaround, but may be good to have something generic to do that:

sebv@cf21173#L1R55

grunt jshint cannot find .jshintrc files in Windows cmd

I've got a project with several .jshintrc spreaded in different folders.

When I run jshint command directly, the corresponding .jshintrc file will be found and the code linting works as expected.

When I run grunt jshint however, It fails to find any .jshintrc. Hence produces loads of error messages because I've got asi enabled but the default is false.

My system is very old. Windows XP with cmd.exe. grunt jshint doesn't work in neither cmd.exe nor Git Bash.

$ jshint --version
jshint v2.1.3

$ grunt --version
grunt-cli v0.1.9
grunt v0.4.1

The version of grunt-contrib-jshint is the latest, 0.6.0

Reporter option doesn't find npm installed reporter module (only paths)

I use a custom reporter that I've pushed and installed over npm but it seems grunt-contrib-jshint doesn't find it if I only pass the module name:

This doesn't work:

'jshint-path-reporter'

Instead I have to use a path into the node_modules folder:

'./node_modules/jshint-path-reporter'

Not a major issue but it seems odd since the reporters are modules and they resolve with the modules package.json's main file (I didn't need to append 'index.js')

readability of JSHint output

I have some ideas for improving the readability of the logged output.

(Pretend there are nice colors)

Before:

[L1:C1] W097: Use the function form of "use strict".
'use strict';
[L474:C54] W008: A leading decimal point can be confused with a dot: '.5'.
                            .style("fill-opacity", .5)
[L717:C50] W040: Possible strict violation.
                            var path = d3.select(this),

After:


  1 |   'use strict';
         ^ Use the function form of "use strict". [W097]

474 |   .style("fill-opacity", .5)
                               ^ A leading decimal point can be confused 
                                 with a dot: '.5'. [W008]

717 |  var path = d3.select(this),
                            ^ Possible strict violation. [W040]

Thoughts?

If it makes sense to do so I'll create a pull request. I might put the formatting code in a separate module so it can be reused by other tools like jslint.

Unable to read ".jshintrc" file

I'm working on an AngularJS app, using the grunt to help me run the dev server, and attempt to package the thing for distribution. I'm very new to the entire tool-chain, grunt, yeoman, and AngularJS itself, and while I've managed to get to the point where I have something worth building, I'm still a bit dis-oriented with all of the tools involved.

My issue is that when I try to run grunt, or grunt jshint:all I get the following error:

Warning: Unable to read ".jshintrc" file (Error code: ENOENT). Use --force to continue.  

Here's the verbose output that I was able to grab:

Running "jshint:all" (jshint) task
Verifying property jshint.all exists in config...OK 
Files: Gruntfile.js, app/scripts/app.js, app/scripts/controllers/main.js, 
app/scripts/controllers/onepod.js, app/scripts/controllers/pods.js, 
app/scripts/services/podlist.js -> all 
Reading .jshintrc...ERROR 
Warning: Unable to read ".jshintrc" file (Error code: ENOENT). Use --force to continue.   
Aborted due to warnings.

I was able to get something built by forcing it, but it wasn't completely successful, and I'd like to figure out what's going wrong here. Unfortunately, I have no clue which .jshintrc file is the issue exactly, as there are at least a dozen found within the project, and the error isn't terribly specific. Another note, I created was using Intellij for a bit with the project, it was created in the command line, but Intellij managed it. Not sure if that could have screwed something up. I am running grunt-contrib-jshint v0.3.0, grunt v0.4.1, grunt-cli v0.1.9 and nodejs v0.10.8.

Any help would be much appreciated!

$ grunt [test] failing ..

hi there..
i've done a fresh checkout from the git repository but..

$ grunt test

Running "nodeunit:tests" (nodeunit) task
Testing jshint_test.js...F......
>> jshint - defaultReporterErrors
>> Error: Expected 3 assertions, 1 ran
>> at process.startup.processNextTick.process._tickCallback (node.js:244:9)

>> jshint - defaultReporterErrors
>> TypeError: Cannot read property 'length' of null
>> at exports.jshint.defaultReporterErrors (test/jshint_test.js:74:57)
>> at stdoutEqual (test/jshint_test.js:28:3)
>> at Object.exports.jshint.defaultReporterErrors (test/jshint_test.js:70:5)
>> at process.startup.processNextTick.process._tickCallback (node.js:244:9)

Warning: 2/15 assertions failed (48ms) Use --force to continue.

Aborted due to warnings.

But the 'travis build' succeeds.. How can that be ?

I'm running this on OSX with following setup.

grunt --version
grunt-cli v0.1.6
grunt v0.4.1

node --version
v0.8.16

Best
David

Use the function form of "use strict".

What's wrong here?


'use strict';

/* Filters */

angular.module('myApp.filters', []).
filter('interpolate', ['version', function(version) {
return function(text) {
return String(text).replace(/%VERSION%/mg, version);
};
}]);

not respecting in-code comment configuration options

I'm using yeoman generators with grunt in my workflow. In my .jshintrc file

I have //jshint unused: false, evil: true for example in my app.js file. When I run jshint app.js I don't get any errors; however, when I run grunt jshint I get errors "eval is evil" and " is defined but never used".

Add support for .jshintignore

cf. gruntjs/grunt#326.

I don't have time or experience with grunt to incorporate it into this repository, but here is what I used locally to make it work. It isn't perfect, see https://github.com/jshint/jshint/blob/master/src/cli/cli.js for the exact method of how jshintignore is utilised.

The main difference is that JSHint excludes along the way, whereas with grunt.file it appears one has to first expand everything and then add a bunch if ! entries. There is probably a better way, though even for a large repository it still runs in under a second for me.

    var get = {
            // Get list of minimatch patterns that should be excluded
            // from JSHint.
            jshintExclude: function () {
                var ignores = grunt.file.read( '.jshintignore' );
                if ( ignores ) {
                    ignores = ignores.split( '\n' )
                        .filter( function ( ignore ) {
                            return !!ignore.trim();
                        } )
                        .map( function ( ignore ) {
                            if ( ignore.slice( -3 ) === '.js' ) {
                                return '!' + ignore;
                            }
                            if ( ignore.slice( -1 ) !== '/' ) {
                                ignore += '/';
                            }
                            return '!' + ignore + '**/*.js';
                        } );
                    get.jshintExclude = function () { return ignores; };
                    return ignores;
                }
                return [];
            }
    };

    grunt.initConfig({
        jshint: {
            options: {
                jshintrc: '.jshintrc'
            },
            all: [ '**/*.js' ].concat( get.jshintExclude() )
        },

Checkstyle reporter sometimes outputs unused variable warnings, even what that option is not used

  1. Set up the jshint task options without "unused": true (or explicitly set it to false), a single input file, and the "checkstyle" reporter.
  2. Run the task and notice that the Checkstyle XML has no unused variable warnings in it.
  3. Add a second input file and run the task again.
  4. Notice that the XML now has unused variable warnings, but only from the second file.

Example setup: https://gist.github.com/Lugribossk/5709321

This is with Grunt v0.4.1 and grunt-contrib-jshint v0.6.0.

Use JSHINT cli directly

Is there any reason we don't just use the jshint cli directly? Meaning do this:

var jshintcli = require('jshint/src/cli/cli');
var gruntReporter = function(results, data) {
  /* Output results in a grunt like format */
};
jshintcli.run({
  args: this.filesSrc,
  extensions: 'js',
  reporter: options.reporter || gruntReporter
});

Or is require('jshint/src/cli/cli') considered a bad practice? If so I'd like to send jshint a PR to expose that to require('jshint').cli. This seems like it would fix all the requests to re-implement jshint things within this task.

This would fix #34, #33, #31, #13 and #8.

Allow comments in .jshintrc

I know this is a minor issue but we all love to put comments in our config files :)

Currently the task just tries to load the .jshintrc as JSON so any comments in the file would cause an error.

In fact jshint itself does allow comments in the file, it simply strips them before trying to load JSON.

I think it makes sense to do the same,

  • for users' convenience
  • for compliance with the original tool (that defines the file format anyway)

Upgrade jshint dependency

Right now v0.9.1 of jshint is being used which unfortunately doesn't include the fix for jshint/jshint#494. Which means I have errors for escapements that shouldn't be errors.

Is there anyway you can bump the version so I don't get errors for these anymore? Current version of jshint in npm is v1.1.0.

Thanks!

Allow take options from a YAML file

like with JSON jshintrc but in YAML (to allow comments and such). Grunt should support YAML.

If you are interested I will submit a pull request.

Issues with concat / jshint combination

As descriped here there are issues with jshint + before and after concat. The solution provided didn't really seem like the sleekest thing around. Is there any way to make my gruntfile check before: and after: in a single run? I just do the following every time now.

  grunt
  grunt

This just doesn't feel right.

EDIT: I just realized the right method was editing the grunt.registertask file. This issue can be closed, I was being silly.

Strange, Undocumented Properties Out in the Wild

I'm finding some strange properties to go alongside jshint.options out in the wild in the grunt contrib community, and I'd really like to know some more about what it is they are meant to do. You can see some here: https://github.com/jsoverson/grunt-template-jasmine-requirejs/blob/master/Gruntfile.js

  grunt.initConfig({
    // Task configuration.
    jshint: {
      options: {
        node : true,
        curly: true,
        eqeqeq: true,
        immed: true,
        latedef: true,
        newcap: true,
        noarg: true,
        sub: true,
        undef: true,
        unused: true,
        boss: true,
        eqnull: true,
        globals: {}
      },
      gruntfile: {       //Undocumented
        src: 'Gruntfile.js'
      },
      lib_test: {        //Undocumented
        src: ['src/**/*.js']
      }
    });

Attempts to consult the source on how you are using them has led me to believe that you are not using them anywhere - I couldn't find anything in your scripts that utilize these properties. Am I mistaken? Is there a purpose for them? How many other undocumented properties are there/am I not finding my way to proper documentation?

Thanks,

  • Keith

verbose flag incompatible with reporters

When grunt is run in verbose (-v) mode, reporters get passed a grunt-specific dump of options that breaks the output.

Pull request fix incoming (Still getting the hang of github, please forgive me if I tag it incorrectly and make a new issue.)

Seen with grunt 0.4.1 and grunt-contrib-jshint 0.6.0

"ignores" parameter is not being used

I'm trying to run jshint through grunt with:

  grunt.initConfig({
    pkg: grunt.file.readJSON( "package.json" ),
    jshint: {
      ignores: [
        "src/wrapper/*.js"
      ],
      files: [
        "Gruntfile.js",
        "make.js",
        "src/**/*.js"
      ]
    }
  })

but no matter whether I place ignores before or after files, the files in the wrapper dir are still loaded and parsed by jshint. Can the README.md be extended for the ignores parameter with an example snippet to show the use of files in combination with ignores, so that I (and others like me =) can see a proper code example that we can use in our own grunt files?

Problem setting up configuration file for 0.4.0rc2

I wanted to try out the latest version of Grunt (currently 0.4.0rc2) but I can't seem to run a basic jshint on my files.

You can see my project repo/set-up here: https://github.com/Integralist/Grunt-Boilerplate

Specifically the Gruntfile content looks like this...

module.exports = function(grunt) {

  /*
      Grunt set-up:
        npm install -g grunt-cli
        npm install -g grunt-init
        npm init

      Requirements: 
        npm install [email protected] --save-dev
        npm install grunt-contrib-watch --save-dev
        npm install grunt-contrib-jshint --save-dev
        npm install grunt-contrib-uglify --save-dev
        npm install grunt-contrib-requirejs --save-dev
        npm install grunt-contrib-sass --save-dev
        npm install grunt-contrib-imagemin --save-dev
        npm install grunt-contrib-htmlmin --save-dev

   */

  // Project configuration.
  grunt.initConfig({

    pkg: grunt.file.readJSON('package.json'),

    uglify: {
      options: {
        banner: '/*! <%= pkg.name %> | <%= pkg.version %> | <%= grunt.template.today("yyyy-mm-dd") %> */\n'
      },
      dist: {
        src: './<%= pkg.name %>.js',
        dest: './<%= pkg.name %>.min.js'
      }
    },

    jshint: {
      options: {
        curly: true,
        eqeqeq: true,
        immed: true,
        latedef: true,
        newcap: true,
        noarg: true,
        sub: true,
        undef: true,
        boss: true,
        eqnull: true,
        browser: true,

        globals: {
          require: true,
          requirejs: true,
          jQuery: true,
          console: true,
          define: true
        }
      },
      /*
          In case there is a /release/ directory found, we don't want to lint that 
          so we use the ! (bang) operator to ignore the specified directory
       */
      all: ['Gruntfile.js', 'app/**/*.js', '!app/release/**']
    },

    // Run: `grunt watch` from command line for this section to take effect
    watch: {
      files: '<%= jshint.all %>',
      tasks: 'jshint' // add space separated list of items to watch
    },

    requirejs: {
      compile: {
        options: {
          baseUrl: './app',
          mainConfigFile: './app/main.js',
          dir: './app/release/',
          modules: [
            {
              name: 'main'
            }
          ]
        }
      }
    }

  });

  // Load NPM Tasks
  grunt.loadNpmTasks('grunt-contrib-watch');
  grunt.loadNpmTasks('grunt-contrib-jshint');
  grunt.loadNpmTasks('grunt-contrib-uglify');
  grunt.loadNpmTasks('grunt-contrib-requirejs');
  grunt.loadNpmTasks('grunt-contrib-sass');
  grunt.loadNpmTasks('grunt-contrib-imagemin');
  grunt.loadNpmTasks('grunt-contrib-htmlmin');

  // Default Task
  grunt.registerTask('default', ['jshint']);

  // Release Task
  grunt.registerTask('release', ['requirejs']);

};

...but if I run either grunt or grunt jshint I get the error message: Warning: Cannot call method 'forEach' of undefined Use --force to continue

Does anyone know what I can do to get this working as the grunt-contrib-jshint GitHub page doesn't really help much as in way of example.

Thanks.

"predef" in options throws error Object #<Object> has no method 'forEach'

I have a .jshintrc that has "predef": {}. When I grunt jshint, it returns

Warning: Object #<Object> has no method 'forEach' Use --force to continue.

It looks like this is related to how the grunt task updates .predef from an Array to the .globals Object. In my case predef is an Object, not an Array.

Include jshint error/warning code in message

One feature I have found useful in the jshint command-line interface is the ability to include the error/warning code in each message. For example, the code for "Line is too long" is "W101". Since jshint allows messages to be suppressed at the individual message code level (via comments in the file) and thus be more granular than the named options allow, having the code in the output message is useful for knowing what code to use.

Currently, grunt-contrib-jshint doesn't display the code anywhere in the output text.

Use `dest` for custom reporter output

Now that this task supports jshint reporters it seems to make sense to utilize dest to output those reports to a file as suggested in #46. The jshint equivalent being:

$ jshint --reporter=checkstyle myfile.js > myreport.xml

This breaks backwards compatibility though as users (including this plugin's Gruntfile) does this currently:

jshint: {
  all_files: [
    'Gruntfile.js',
    'tasks/**/*.js',
    '<%= nodeunit.tests %>'
  ],
}

Which with this change would try and write the report to all_files.

/cc @cowboy

.jshintignore doesn't work with wildcards

If grunt-contrib-jshint is configured in the Gruntfile as follows:

grunt.initConfig({
    jshint: {
        all: ['**/*.js'],
        options: {
            jshintrc: '.jshintrc'
        }
    }
});

and a .jshintignore has the following contents:

node_modules/**

running grunt jshint results in the files in node_modules still being linted.

JShint has issues with tilde (~) in some contexts

When I try to use tilde in the context of !!~string.indexOf('otherstring'), JSHint via Grunt gives me the following lint error

Unexpected '~'

Is this usage really frowned upon or is this an unaccounted case for JSHint?

Allow specifying the JSHINT function

Would there be any possibility of allowing us to provide our own JSHINT fn to the task, perhaps something along the lines of:

jshint: {
    options: {
        jshint: require( "../my-custom-jshint" )
    }
}

The use case for it is so that if there's a long-standing jshint issue that doesn't seem like it's getting traction for whatever reason ( jshint/jshint#935 in my case ), we could fork jshint and provide it to the grunt task without also having to fork it.

Errors when running against targets

I am attempting to use the syntax shown in the README to make violations halt release builds only, but getting a weird error when using the targets. My syntax is as follows:

jshint: {
    options: {
        jshintrc: '.jshintrc'
    },
    dev: {
        options: {
            force: true
        },
        files: ['./Gruntfile.js', '<%= cfg.js_src %>', '!<%= cfg.libs_dir %>/**']
    },
    release: {
        files: ['./Gruntfile.js', '<%= cfg.js_src %>', '!<%= cfg.libs_dir %>/**']
    }
}

and the error I get is Warning: Cannot use 'in' operator to search for 'src' in ./Gruntfile.js� Use --force to continue. with an exit code of 3.

Array index is hardcoded in jshint.js L90

If I have Gruntfile.js like this:

grunt.initConfig({
    jshint: {
      all: ['Gruntfile.js', 'config.js', 'src/**/*.js'],
      options: {
        jshintrc: '.jshintrc',
        force: true
      }
    }
  });

And, in example, missing semicolon in src/foo.js, then i see console output like this:

jshint

So, the problem is what I instead of file with error I always see the first file in array.

This is possible because array index is hardcoded here: https://github.com/gruntjs/grunt-contrib-jshint/blob/master/tasks/lib/jshint.js#L90

How can I hide specific errors/warnings like "W034"?

In the following example ERROR, I would like to ignore these.

Linting app/scripts/app.js...ERROR
[L24:C9] W015: Expected '}' to have an indentation at 11 instead at 9.
        };

According to the docs for jshint, it is possible to switch errors off based on a code, ie [+|-]W015. The docs mention:

Then, to hide this warning, just add the following snippet to your file:

/* jshint -W034 */

Is there a way to specify this in the options block?

jshint: {
    options: {
        jshintrc: '.jshintrc',
        curly: false  
    },
    all: [
        'Gruntfile.js',
        '<%= yeoman.app %>/scripts/{,*/}*.js'
    ]
},

Reporter option

When using the new reporter option, a lot of errors are output with the message : Bad option: 'reporter'.

here is my grunt config :

jshint: {
      options: {
        "reporter" : "checkstyle",
        "force" : true,
        "browser": true,
        "node": false,
        "globalstrict" : true,
        globals: {
          angular : true,
          console : true,
          i18n : true,
          $ : true
        }
    },
    src: [
      '<%= dirs.js %>/**/*.js'
    ]

By the way, is it possible to specify a xml file output for the checkstyle reporter?

support jshint-cli reporter option

It would be nice to have the jshint reporter option available as part of the grunt task. This could then be used in CI like jenkins to display checkstyle reports.

Please npm publish

Current npm version is 0.2.0, I would love to have 0.3.0.

(I especially need fc6397e)

Greetings
Johannes

Object #<Object> has no method 'options'

I am seeing the following error trying to use the grunt jshint task.

TypeError: Object #<Object> has no method 'options'
    at Object.module.exports (/Users/patricklocal/Projects/personal/grunt/node_modules/grunt-contrib-jshint/tasks/jshint.js:18:24)
    at Object.task.registerMultiTask.thisTask (/usr/local/share/npm/lib/node_modules/grunt/lib/grunt/task.js:109:15)
    at Object.task.registerTask.thisTask.fn (/usr/local/share/npm/lib/node_modules/grunt/lib/grunt/task.js:58:16)
    at Task.<anonymous> (/usr/local/share/npm/lib/node_modules/grunt/lib/util/task.js:343:36)
    at Task.<anonymous> (/usr/local/share/npm/lib/node_modules/grunt/lib/util/task.js:319:9)
    at Task.<anonymous> (/usr/local/share/npm/lib/node_modules/grunt/lib/util/task.js:346:11)
    at Task.start (/usr/local/share/npm/lib/node_modules/grunt/lib/util/task.js:359:5)
    at Object.grunt.tasks (/usr/local/share/npm/lib/node_modules/grunt/lib/grunt.js:143:8)
    at Object.module.exports [as cli] (/usr/local/share/npm/lib/node_modules/grunt/lib/grunt/cli.js:36:9)
    at Object.<anonymous> (/usr/local/share/npm/lib/node_modules/grunt/bin/grunt:19:14)

I have installed the grunt jshint plugin with npm install grunt-contrib-jshint --save-dev and my grunt.js file looks like this...

/*global module:false*/
module.exports = function(grunt) {

  // Project configuration.
  grunt.initConfig({
    jshint: {
      all: ['test.js']
    }
  });

  grunt.loadNpmTasks('grunt-contrib-jshint');

};

Any advice?

fail noise/alert with force

I have a custom task which runs ['connect', 'watch'] to run a server and run jshint when files are changed. I set watch to use force, so a failed lint wouldn't stop the server:

  watch: {
    scripts: {
      files: ['Gruntfile.js', 'src/*.js', 'src/**/*.js'],
      tasks: ['jshint:force'],
      options: {
        nospawn: true
      }
    }
  },
  jshint: {
    all: ['Gruntfile.js', 'src/*.js', 'src/**/*.js'],
    force: {
      options: { force: true },
      files: { src: ['Gruntfile.js', 'src/*.js', 'src/**/*.js'] }
    }
  },

In 0.3 there would be an alert noise and the terminal icon would bounce once. It seems like in 0.4 force is making it fail quietly. Can I get the noise back without stopping the server?

jshintrc option does not merge with task/target level options

As of 0.4.3, the jshintrc option contents are not merged with task/target options. This seemed to work as expected in 0.3.0.

Sample config (abbreviated):

jshint: {
  options: {
    jshintrc: '.jshintrc'
  },
  gruntfile: {
    options: {
      globals: {
        'module': true,
        'require': true
      }
    },
    src: ['Gruntfile.js']
  }
}

In the above scenario, the gruntfile.options object is ignored.

report file trouble for 'jshint' reporter with --verbose

I have to following setup:

    jshint: {
        rjs: {
            src:[
                'requirejs-test/main/src/**/*.js'
            ],
            options: {
                jshintrc: '.jshintrc-rjs',
                reporter: 'jslint',
                reporterOutput: 'reports/jshint.xml'
            }
        },
    ...

running with --verbose flag pollutes the reported xml file with console output (including colour tags) like ..

// reports/jshint.xml
Reading .jshintrc-rjs...�[32mOK�[39m
Parsing .jshintrc-rjs...�[32mOK�[39m
JSHint options: �[36mbitwise=false�[39m, �[36mcamelcase=false
:true,"beforeEach":true,"afterEach":true,"expect":true,"waitsFor":true}�[39m
<?xml version="1.0" encoding="utf-8"?>
<jslint>
 ...

Output is very verbose and hard to read

So I've notice with the switch to 0.5 I'm now getting super verbose, hard to grok output. Essentially what is happening is that there is a call to cli.run foreach file is being linted, which creates a report per file. If a file happens to be ignore, you get the following for that single file.

0 files linted. Please check your ignored files.

screen shot 2013-05-19 at 1 06 00 pm

Is there a reason for this behaviour?

Ideally reporters could still be utilised with a single call to cli.run and we can get back to the good old days of

screen shot 2013-05-19 at 1 10 19 pm

An alternate implementation

Why?

The existing grunt-contrib-jshint task at the time of this writing is hard to change and has limited unit tests. Rather than work the required large changes into your existing codebase I've decided to make grunt-jshint-bfs.

Significant Differences

Class Based

The units of work have been isolated into different classes rather than a single exposed init function. I believe this makes them easier to test and easier to expose as an API to other consumers.

More Unit Tests

I've tried to make sure the critical parts of the code have test coverage, whereas it seems that unit testing has fallen by the wayside for the contrib task.

Explicit jshint options

I've changed the options structure to be more explicit about what the jshint options are in an effor to separate them from the task specific options. I think this eliminates a lot of complicated option deleting code that is in the grunt-contrib-jshint codebase.

Asynchronous

I, personally, do not think that file reading should be done synchronously via the grunt.file.read function. Although my limited testing hasn't revealed any definitive improvements, I believe each file should be linted asynchronously for performance.

Caches Successful Runs

If we've previously run jshint on a file, it should be cached so that we do not need to lint it again. This is done through a combination of temp file based caching where a hash of the contents, the jshint options and the globals determine whether or not we need to lint the file again. This is on by default, but can be controlled via an option.

Reporter Architecture

While there is an existing Pull Request open (#34) to implement reporters, I am not happy with the hard linking of the reporter files and the hacky overriding of stdout necessary to make them work together nicely. I've ported over the existing xml and checkstyle reporters, and have plans to write a base legacy reporter class that will help others who have existing reporters built port them to be used with this task.

Misc. Style stuff

You guys are 2 space tab kinda guys, I'm a 4 space tab kinda guy. You guys are not fans of white space, I'm a big fan of it. Evidently you are also against single var statements for multiple assignment. I have no problem converting to your style if you want.

Where do we go from here?

I'd love to hear some feedback and have a dialog about the points I've raised above. I've never understood the init function based architecture in use and I'm curious if I might be missing something.

I'd be happy to talk about starting up a new branch with this code and merging it in over time. I'm sensitive to the options structure change and understand that it might be something that a minor version would be incremented for.

As a final option, if all my code just makes you want to vomit in your mouth a little bit, I could just publish this as a separate task and check back in from time to time looking wistfully at your code and thinking of the oh so sweet merging we could have done together.

i believe it should use grunt.log.error instead of grunt.log.writeln

@tkellen - do you remember why you use grunt.log.writeln instead of grunt.log.error here?

// Manually increment errorcount since we're not using grunt.log.error().
grunt.fail.errorcount++;
// Descriptive code error.
pos = '['.red + ('L' + e.line).yellow + ':'.red + ('C' + character).yellow + ']'.red;
if (e.code) {
  code = e.code.yellow + ':'.red + ' ';
}
grunt.log.writeln(pos + ' ' + code + e.reason.yellow);

The reason I ask is I maintain grunt-notify which shows desktop notifications via Growl/OSX Notification/etc when errors happen. This works via grunt.hook into grunt.log.error and grunt.log.warn. Users want to find out about jshint errors without having a terminal visible. See dylang/grunt-notify/issues/5.

I'm thinking about creating a PR that would utilize grunt.log.error or grunt.log.warn. Do you have thoughts on this?

onevar not reported as error

I'm not sure if this is only the onevar setting or if the whole file is not linted for some reason. As you can see from the console ouput, it says it's linted but missses the obvious second var statement. I noticed this in a bigger grunt project but distilled it into a minimal example.

Curiously with the bigger project, in some files (such as the gruntfile itself) it actually does complain about the multiple var statements...

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.