JSHint doesn't respect "indent" option #655

Closed
cowboy opened this Issue Sep 20, 2012 · 20 comments
@cowboy

This seems broken to me. Observed in node-jshint 0.8.1 and 0.9.0 as well as the latest jshint.js from this repo.

http://jsfiddle.net/cowboy/FPXAv/

// Indent specified in comment block.
JSHINT('/*jshint indent: 4*/\nfunction foo() {\nfoo();\n}', {});

JSHINT.errors.length    // 1
JSHINT.errors[0].reason // "Expected 'foo' to have an indentation at 5 instead at 1."

// Indent specified as option (broken?)
JSHINT('function foo() {\nfoo();\n}', {indent: 4});

JSHINT.errors.length    // 0
@cowboy cowboy referenced this issue in gruntjs/grunt Sep 20, 2012
Closed

JSHint Indent option isn't working #442

@nfriedly

Yep, I'm seeing this too. To add some info, when white: true is added, indent does work as expected, but then other things are checked for too.

JSHINT('function foo() {\nfoo();\n}', {indent: 4, white: true});

JSHINT.errors.length    // 1
JSHINT.errors[0].reason // "Expected 'foo' to have an indentation at 5 instead at 1."
@valueof valueof added a commit that closed this issue Sep 25, 2012
@valueof valueof Make 'indent' behaviour consistent.
When 'indent' is used as a comment it implies 'white' option. (see
L1973). This behaviour came to us as JSLint's legacy and is outside
of scope of this commit. This commit simply makes it so the 'indent'
option passed as an argument to JSHINT also implies 'white'.

Fixes GH-655.
80277ef
@valueof valueof closed this in 80277ef Sep 25, 2012
@nschonni nschonni added a commit to nschonni/jshint that referenced this issue Oct 4, 2012
@valueof valueof Make 'indent' behaviour consistent.
When 'indent' is used as a comment it implies 'white' option. (see
L1973). This behaviour came to us as JSLint's legacy and is outside
of scope of this commit. This commit simply makes it so the 'indent'
option passed as an argument to JSHINT also implies 'white'.

Fixes GH-655.
9874ff7
@andreineculau

Was commit 80277ef thought through ? It seems to be a breaking jump from 0.9.0 to 0.9.1, and it also seems to address only indirectly this issue.

The description of white at http://www.jshint.com/docs/ does not overlap with indent.
What is the purpose of white, if it gets set to true whenever indent is set, while white means a whole lot more?

#667
#666

@dcherman

+1 @andreineculau

Either the documentation needs to be updated, or preferably 'indent' would not require/imply 'white' since that's a far stricter set of rules that even the docs say "are not very well documented".

@nybble73

Yes - please separate indent from white...

@mrrena

I'm very glad I found this issue, as removing an indent of 4 definitely stopped white (which is set to false) from misbehaving: was driving us insane.

@felix-ordrin Agreed.

@corsen2000

Same issue here. I want to set indent:4, but then I get errors about not having a space after an "if" and such.

It would be awesome if indent was separated from white.

@bdkjones

+1 for undoing this commit.

Please see my full comment on a related issue --> #667 (comment)

@cowwoc

+1 for separating ident and white. Is there a separate issue open for this or do you plan on reopening this issue?

@dscape

Should this be reopened (am i doing something wrong here?)

> JSHINT = require('jshint').JSHINT
{ [Function]
  addModule: [Function],
  data: [Function],
  jshint: [Circular],
  errors: [],
  undefs: [],
  internals: [],
  blacklist: {},
  scope: '(main)' }
> JSHINT('/*jshint indent: 60*/\nfunction foo() {\nfoo();\n}', {});
true
>
undefined
> JSHINT.errors.length
0
> JSHINT.errors
[]
>
undefined
> JSHINT('function foo() {\nfoo();\n}', {indent: 60});
true
>
undefined
> JSHINT.errors.length
0
@nfriedly

@dscape, the impression that I have is that jshint doesn't want to support checking "style" things like indentation. So, I've started running jsbeautify in rewrite mode before committing, and running it in verify-only mode in the CI loop.

@rwaldron
JSHint member

@dscape ^^ what they said. JSHint recommends using JSCS for style related tooling. @mikesherov will gladly help you with anything you need to get started.

@mikesherov

@dscape let me know if you need anything to help you get started. One of the easiest ways to begin using JSCS is to use one of our presets. I'd recommend crockford to gain the white functionality of JSHint, or you can simply use the JSCS validateIndentation option. Let me know if you have any more questions!

@dscape
@mikesherov

@dscape it was removed in 2.5.0

@dscape
@mikesherov

@dscape lots of people are using JSHint for program correctness and JSCS for style these days. There are a bunch of beautifiers and style linters out there, please consider JSCS as one of them :-)

@nfriedly

@dscape I don't have a blog post detailing everything, but here's an example of using jsbeautifier in both rewrite and verify modes (via grunt): https://github.com/nfriedly/Coin-Allocator/blob/master/Gruntfile.js#L17

My git pre-commit script is just npm test which calls grunt test which then blocks the commit if the files aren't beautified. Then I manually run grunt beautify to re-write the files with proper formatting if any of them need it. (I don't like the idea of it automatically re-writing my files, it just seems a little too brittle.)

@dscape
@jugglinmike jugglinmike added a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014
@valueof valueof Make 'indent' behaviour consistent.
When 'indent' is used as a comment it implies 'white' option. (see
L1973). This behaviour came to us as JSLint's legacy and is outside
of scope of this commit. This commit simply makes it so the 'indent'
option passed as an argument to JSHINT also implies 'white'.

Fixes GH-655.
0705442
@niedzielski niedzielski added a commit to niedzielski/jshint that referenced this issue Nov 12, 2015
@niedzielski niedzielski Remove unsupported indent option
Indent is no longer supported as of v2.5.0[0].

[0] jshint#655
ff5c27d
@jugglinmike jugglinmike added a commit that referenced this issue Nov 21, 2015
@niedzielski niedzielski [[DOCS]] Remove unsupported indent option
Indent is no longer supported as of v2.5.0[0].

[0] #655
56527c7
@jugglinmike jugglinmike added a commit to jugglinmike/jshint that referenced this issue Jan 2, 2016
@niedzielski niedzielski [[DOCS]] Remove unsupported indent option
Indent is no longer supported as of v2.5.0[0].

[0] jshint#655
b8d4b40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment