clutchski / coffeelint

Lint your CoffeeScript.
http://www.coffeelint.org
Other
1.18k stars 172 forks source link

Support for tab indented literate files added #559

Closed KrysKruk closed 8 years ago

KrysKruk commented 8 years ago

This PR adds support for trailing tab whitespace in literate files. See #557 for more.

swang commented 8 years ago

Thanks. It does seem like Literate CoffeeScript should indeed support 4 spaces or 1 tab. However I just tried it out on this test case.

This is a function that returns 'hello'
    () ->
        "hello"

Wow that was easy!

And it produces

(function() {

}).call(this);

Is this working on your end? I'm running node 4.0.0 (tried node 0.12.0 as well) running the latest coffeescript.

I looked around CoffeeScript's source code and it seems like a change made 3 years ago caused this so you may have to file an issue with them to get it fixed. After that I can merge in this code.

https://github.com/jashkenas/coffeescript/commit/52ca531b7c9f1fc6f3ba76d2ac2527c41f9ece68

KrysKruk commented 8 years ago

Hi @swang Thank you for feedback!

I think your test case works well, it should produce an empty function, because Markdown requires one extra empty line before indented code. It's easy to test on GitHub comments. If I add one blank line before the code, everything works well. What do you think? Should coffeelint support this test case?

swang commented 8 years ago

Ah I did not know that you needed to add an additional line since it doesn't seem to be mentioned in any of the docs. I assumed the indentation was enough.

KrysKruk commented 8 years ago

Thank's for merging this into the main branch. Do you know, when new version will be published in NPM?

swang commented 8 years ago

I just released 1.15.1 that includes your PR.

KrysKruk commented 8 years ago

@swang Thank you again for quick response. Have a nice day!