palantir / tslint-react

:orange_book: Lint rules related to React & JSX for TSLint.
Apache License 2.0
749 stars 76 forks source link

Implement jsx-equals-spacing rule #104

Closed bolatovumar closed 7 years ago

bolatovumar commented 7 years ago

Issue #103

Ported over the rule from eslint-plugin-react. Included the exact same tests as in that plugin.

No automatic fixer because I don;t know how it works and never use it myself. I reckon it would be really easy to add the fixer if someone wanted to do.

adidahiya commented 7 years ago

I strongly recommend adding a fixer here, it should be pretty simple. You just have to add a text replacement for " " -> "".

Can you look into getting CI status reported here?

bolatovumar commented 7 years ago

hey @adidahiya, I'm running into some weird issues with the fixer.

Here is what the test output looks like without the fixer applied: no_fix

It looks like what you would expect to see.

Here is what the test output looks like with the fixer applied: with_fix

To me it looks like the test should pass yet it fails. Any pointers here?

As per the CircleCI status, I did build my fork through the CircleCI's online interface. It only showed me the master branch, however. Not sure where I'm going wrong here.

adidahiya commented 7 years ago

@bolatovumar can you push your in-progress fixer code here so I can take a look?

bolatovumar commented 7 years ago

@adidahiya, sorry, it took me a while. Add my failing WIP fixers above.

The commands I'm running to test the fixers are (from the root of the repo after running npm run build): ./node_modules/.bin/tslint -r ./build/rules/ --test ./test/rules/jsx-equals-spacing/always ./node_modules/.bin/tslint -r ./build/rules/ --test ./test/rules/jsx-equals-spacing/never

In both cases it looks like the console output I'm getting looks something like this:

capture

Hope this is enough detail.

IllusionMH commented 7 years ago

@bolatovumar I've seen something similar when forgot to add empty line at the end of a .fix file.

UPD. Checked changes from PC and now I see that this should be the case: IIRC all lines that mark errors and starts with ~ or [0] etc. should be skipped. All other empty lines should be exactly same as in .lint file. You can't see diff because line breaks aren't visible and can't be highlighted with color

bolatovumar commented 7 years ago

@IllusionMH, alright, adding and removing line breaks seems to help