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

'indent' triggers an implicit 'white' #667

Closed
brettstimmerman opened this Issue Oct 3, 2012 · 6 comments

Comments

Projects
None yet
6 participants
@brettstimmerman
Copy link

brettstimmerman commented Oct 3, 2012

The current docs for 'indent' and 'white' don't mention that 'indent' triggers an implicit 'white'. This relationship should be mentioned in the docs, especially since 'white' is considered legacy while 'indent' is not.

What is the reasoning behind why 'indent' triggers 'white'? It seems (on the surface) as though they could be independent.

@goatslacker

This comment has been minimized.

Copy link
Member

goatslacker commented Oct 3, 2012

There's no use for indent unless you turn on white. Ideally we'd remove indent and just have white be an Object where you can set the indent level and other properties.

@brettstimmerman

This comment has been minimized.

Copy link

brettstimmerman commented Oct 4, 2012

I guess I was picturing a scenario where I care about indentation but don't care about if(...) vs if (...). But, I understand if it's complicated or otherwise too much hassle to separate them to support fine-grained whitespace rules.

Either way, I still think that the docs should be updated to reflect the relationship. I think @antonkovalyov is already aware based on a Twitter conversation I had.

@bkw

This comment has been minimized.

Copy link
Contributor

bkw commented Oct 29, 2012

I'd vote to keep indent like it is and instead split the ominous "white" option into more granular ones (spaces around if, for, function). Imho "indent" on it's own is much more useful and less controversial than all the other rules implied by "white".

@bdkjones

This comment has been minimized.

Copy link

bdkjones commented Nov 8, 2012

The original poster is correct --- stuff like this needs to be documented.

I write CodeKit, an app that integrates JSHint. CodeKit has always passed a value for the 'indent' parameter because doing so never caused any problems. Prior releases of JSHint did not "magically" flip on options just because an indent parameter was specified.

The latest JSHint release, however, changed that behavior. Since CodeKit always passes an indent value, the 'white' option is now being enabled behind my users' backs and they're seeing tons of warnings on code that used to pass JSHint just fine. (I've gotten LOTS of emails in the past 24 hours about it.)

Now, I disagree with the change that automatically flips on the 'white' option. I think that was a bad decision.

HOWEVER, the real problem is that JSHint changes are so poorly documented, I had no idea that this was going on until tons of people emailed me to complain. The "version history" file in the JSHint download was last updated over a year ago and you guys don't put together any sort of release notes for the changes you make between versions.

That needs to change. JSHint is (rightly) a very popular tool that's integrated into apps all over the place. People rely on your work. You guys need to do a better job documenting changes so that those of use who integrate JSHint can keep up. Docs aren't sexy. No one likes writing them. They are, however, extremely important --- especially when you randomly make changes like this that break backwards compatibility in a big way.

None of this comment is meant disrespectfully. I love the work you guys do on JSHint and I use the tool routinely in my own coding. It's simply frustrating that changes are so poorly documented. Please make this a priority moving forward.

@jakub-g

This comment has been minimized.

Copy link

jakub-g commented Nov 26, 2012

I agree with @bkw -- we use a code formatter that has a lot of granular options like spaces after function name, after anonymous functions and lots more, while there's only one option in JSHint for this. Due to this we've got a zillion of warnings after upgrading from 0.9.0 to 0.9.1.

+1 for splitting white into more options.

@valueof valueof closed this in e99974f Dec 5, 2012

@bkw

This comment has been minimized.

Copy link
Contributor

bkw commented Dec 5, 2012

great, thanks @antonkovalyov!

jugglinmike added a commit to jugglinmike/jshint that referenced this issue Oct 21, 2014

Split options 'indent' and 'white'.
This commit splits options 'indent' and 'white'. Before, 'indent' was
implying 'white' but many people want to use the former without the
latter. New behavior is that indentation warnings will be triggered
only if: user explicitly set an 'indent' option or user set a 'white'
option.

Closes jshintGH-667.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment