schorfES / grunt-lintspaces

A Grunt task for checking spaces in files.
https://npmjs.com/package/grunt-lintspaces
MIT License
31 stars 9 forks source link

Indentation detection not working well in html.erb files #15

Closed ghost closed 10 years ago

ghost commented 10 years ago

I’ve setup lintspaces with the following:

lintspaces: {
            html: {
                src: [
                    '**/*.html.erb'
                ],
                options: {
                    newline: true,
                    trailingspaces: true,
                    indentation: 'spaces',
                    spaces: 2
                }
            },
        }
}

but when I run it, it does not detect any problems even though all my html.erb files are indented with 4 spaces instead of 2. I’m guessing that’s because 2 is a mulltiple of 4? Any way around that?

The newline detection does work though...

schorfES commented 10 years ago

Lintspaces doesn't take care about the content or language in a certain file. It's also valid to have 4 spaces in a file, when a setting for 2 spaces of indentation is used. You're right it depends on multiple values (See: Validator.js Line 318).

What do you expect to get as output? It's not invalid... I think lintspaces can only guess if there its an incorrect indentaiton, but cannot prompt an error.

ghost commented 10 years ago

Yeah, I don’t think there’s an easy solution for HTML files. Was trying to figure out a solution that’d be a little more robust, but I don’t have anything to propose.

I’ll see if I can figure out something, and if not your tool already helped me to get rid of trailing whitespace and most of the indentation inconsistencies that the project I just jumped into had.

I’m guessing each line should always have either one more unit of indentation, one less unit of indentation, or the same amount of indentation than the previous line. If it’s not the case, then the indentation is probably bad. But you probably thought way more about this already than I did.

ghost commented 10 years ago

It’s just a bit annoying that 4 spaces and 2 spaces are the most common for indentation. So in a lot of cases, it will fail to detect the problem in indentation.

schorfES commented 10 years ago

Initially this task was intended to detect a mixup of used tabs and spaces in a file. I can think about a rule to guess if the used indentation is correct, based on your rule above:

The indentation of the current line is correct when:

But as described above, the result can only end up in an info output. For example those lines are valid code and the indentation is also valid - its just a matter of taste:

var orientation = (width > height)
                ? 'landscape'
                : 'portrait';
ghost commented 10 years ago

Yes, very true. I think detecting tabs and off by one indentation errors will have to do for html.  The rest will have to be caught in code reviews :)

ghost commented 10 years ago

I’m closing this one for now.

schorfES commented 10 years ago

I've added a new Issue to implement the guess feature...

ghost commented 10 years ago

@schorfES ok cool. I might have a crack at it when I have more freetime. Some of my coworkers really like aligning arguments so I might not be able to use it on this project though... I always find that it looks weird.