clutchski / coffeelint

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

Fixes to support CS2 #596

Closed swang closed 6 years ago

swang commented 7 years ago

Still 3 broken tests, 1 test is due to possibly to bug described here: jashkenas/coffeescript#4463

GeoffreyBooth commented 7 years ago

https://github.com/jashkenas/coffeescript/issues/4463 has been fixed in https://github.com/jashkenas/coffeescript/issues/4485

renchap commented 7 years ago

The brand new JSX syntax in CS2 also needs to be handled.

GeoffreyBooth commented 7 years ago

The JSX syntax is especially important for a linter to handle. In the docs I recommend people always put spaces on either side of < and >, e.g. i < 1 rather than i<1, to disambiguate comparisons from JSX tags. In the compiler we tried to be as lenient as possible about this, to minimize the breaking change for people who have e.g. i<1 in their code, by doing what we can to detect when < is being used to compare instead of delimit a JSX tag; in this example, the 1 is a tip-off, because JSX tags can’t begin with numbers. But while the compiler tries to be forgiving, the linter should be the opposite, throwing warnings wherever a < or > appears to be used in a comparison (such as when there’s no matching closing < or > symbol).

kieran commented 7 years ago

Is this being merged?

renchap commented 6 years ago

Coffeescript 2 is now released: http://coffeescript.org/announcing-coffeescript-2/ It would be awesome to have support for it in coffeelint soon :)

swang commented 6 years ago

All test pass. If you want to try out CS2 support feel free to point to this branch. It is late here, so I want to wait until I have time to monitor when the library before publishing to npm.

boris-petrov commented 6 years ago

@swang - thank you for the support. As I wrote here: https://github.com/clutchski/coffeelint/issues/595 - I'm getting an error when installing the cs2 branch. The same error happens with your new commits. Any ideas what's going on?

swang commented 6 years ago

Sorry it's a bit late here so I'm not going to be able to look at this until tomorrow. I'm guessing npm installing a branch was never viable.

boris-petrov commented 6 years ago

No problem, good night, we can wait a bit more. :) Thanks again for the support, we all appreciate it!

It's kind of strange because this file IS there. Also I've npm-installed packages from GitHub branches before... not sure what the issue is.

danielbayley commented 6 years ago

I also ran into errors trying to install with npm using their git:user/repo#branch syntax.

Although I happen to already have a local clone from contributing other PRs, so I checked out the cs2 branch and have been using that via npm link, so far with no errors, and all tests pass for me also. So hopefully @swang, this should be good to go?

boris-petrov commented 6 years ago

@swang - could you merge this and release a new version please? :)