Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Indentation strict and not strict mode. #65

Closed
wants to merge 2 commits into from

3 participants

@ghost

This is so we can ignore issues with coffeelint not correctly
understanding indentation, but still have a tiny little indentation
check for consistency, just that it's a multiple of what we set the
indentation value to.

This is how we fixed #64 #4 and #46 for ourselves.

I'm optimistic because we added it as an option, it might not be to problematic to pull into the core?

Derek Bredensteiner Adding a strict mode for indentation.
This is so we can ignore issues with coffeelint not correctly
understanding indentation, but still have a tiny little indentation
check for consistency, just that it's a multiple of what we set the
indentation value to.
48bb1ad
@thomasf

I'm not sure.. I will look into the indentation linting code first.. If this is to be merged it needs tests and a documentation update as well.

@elspoono elspoono Add test for strict false
Uses the existing test from the indentation topic, it is only testing for indentation that is a multiple of the configured number.
f7675f6
@ghost

Not sure if that sole test I added is sufficient, I will take another stab if told it is not.

Wasn't sure where to do a documentation update, here's a gist https://gist.github.com/4022479 of a suggested modification to http://www.coffeelint.org/#options

@thomasf

Great job!

Add at least one failing and an passing test.

I just began coding coffeelint yesterday myself, I modeled these tests after one of the other tests:
https://github.com/clutchski/coffeelint/blob/master/test/test_no_stand_alone_at.coffee

The thing I'm still most uncertain about is actually the wording "strict" since this relates to a possible bug it's more of a workaround.. I'll think about it and get back coffeelint later this week.

@AsaAyers
Collaborator

I'm going to go ahead and close this. I'm interpreting the lack of activity here for the last year as a lack of need for non-strict mode. Indentation still isn't perfect but has been significantly improved.

@AsaAyers AsaAyers closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 27, 2012
  1. Adding a strict mode for indentation.

    Derek Bredensteiner authored
    This is so we can ignore issues with coffeelint not correctly
    understanding indentation, but still have a tiny little indentation
    check for consistency, just that it's a multiple of what we set the
    indentation value to.
Commits on Nov 6, 2012
  1. @elspoono

    Add test for strict false

    elspoono authored
    Uses the existing test from the indentation topic, it is only testing for indentation that is a multiple of the configured number.
This page is out of date. Refresh to see the latest.
View
3  examples/coffeelint.json
@@ -18,7 +18,8 @@
"indentation" : {
"value" : 2,
- "level" : "error"
+ "level" : "error",
+ "strict" : true
},
"no_implicit_braces" : {
View
17 lib/coffeelint.js
@@ -1,4 +1,4 @@
-// Generated by CoffeeScript 1.3.3
+// Generated by CoffeeScript 1.4.0
/*
CoffeeLint
@@ -52,6 +52,7 @@ CoffeeLint is freely distributable under the MIT license.
indentation: {
value: 2,
level: ERROR,
+ strict: true,
message: 'Line contains inconsistent indentation'
},
no_implicit_braces: {
@@ -499,6 +500,17 @@ CoffeeLint is freely distributable under the MIT license.
if (token.generated != null) {
return null;
}
+ expected = this.config['indentation'].value;
+ if (!this.config['indentation'].strict) {
+ if (numIndents % expected === 0) {
+ return null;
+ } else {
+ context = ("Expected multiple of " + expected + " ") + ("got " + numIndents);
+ return this.createLexError('indentation', {
+ context: context
+ });
+ }
+ }
previous = this.peek(-2);
isInterpIndent = previous && previous[0] === '+';
previous = this.peek(-1);
@@ -511,7 +523,6 @@ CoffeeLint is freely distributable under the MIT license.
previousIndentation = previousLine.match(/^(\s*)/)[1].length;
numIndents -= previousIndentation;
}
- expected = this.config['indentation'].value;
if (!ignoreIndent && numIndents !== expected) {
context = ("Expected " + expected + " ") + ("got " + numIndents);
return this.createLexError('indentation', {
@@ -573,7 +584,7 @@ CoffeeLint is freely distributable under the MIT license.
var i, lastNewLineIndex, lines, t, token, tokens;
lines = (function() {
var _i, _len, _ref, _results;
- _ref = this.tokens.slice(0, this.i + 1 || 9e9);
+ _ref = this.tokens.slice(0, +this.i + 1 || 9e9);
_results = [];
for (i = _i = 0, _len = _ref.length; _i < _len; i = ++_i) {
token = _ref[i];
View
14 src/coffeelint.coffee
@@ -55,6 +55,7 @@ RULES =
indentation :
value : 2
level : ERROR
+ strict : true
message : 'Line contains inconsistent indentation'
no_implicit_braces :
@@ -414,6 +415,18 @@ class LexicalLinter
return null if token.generated?
+ # Only check that the spacing is a multiple of the indentation
+ # value, when in "non strict" mode. This is useful for bypassing
+ # coffeelint indentation terseness.
+ expected = @config['indentation'].value
+ if not @config['indentation'].strict
+ return if numIndents % expected is 0
+ null
+ else
+ context = "Expected multiple of #{expected} " +
+ "got #{numIndents}"
+ @createLexError('indentation', {context})
+
# HACK: CoffeeScript's lexer insert indentation in string
# interpolations that start with spaces e.g. "#{ 123 }"
# so ignore such cases. Are there other times an indentation
@@ -446,7 +459,6 @@ class LexicalLinter
numIndents -= previousIndentation
# Now check the indentation.
- expected = @config['indentation'].value
if not ignoreIndent and numIndents != expected
context = "Expected #{expected} " +
"got #{numIndents}"
View
9 test/test_indent.coffee
@@ -37,6 +37,15 @@ vows.describe('indent').addBatch({
error = errors[0]
assert.equal(error.lineNumber, 2)
+ 'can be not strict' : (source) ->
+ config =
+ indentation:
+ level: 'error'
+ value: 2
+ strict: false
+ errors = coffeelint.lint(source, config)
+ assert.equal(errors.length, 0)
+
'is optional' : (source) ->
config =
indentation:
Something went wrong with that request. Please try again.