Added object-curly-spacing rule #38
Conversation
|
If you'd like to regenerate the rule output file so the tests work, I'll be able to merge this. It's tslint-config-standard/package.json Line 13 in 1767594 |
|
@blakeembrey, done! |
| ERROR: /rules/import-spacing.ts[1, 7]: A space is required after '{' | ||
| ERROR: /rules/import-spacing.ts[1, 22]: A space is required before '}' | ||
| ERROR: /rules/import-spacing.ts[2, 8]: A space is required after '{' | ||
| ERROR: /rules/import-spacing.ts[2, 17]: A space is required before '}' |
There was a problem hiding this comment.
Seems like it's marking the same things as "missing whitespace" above it, we should look at de-duplicating that. Did you want to add any test cases to show this improves coverage?
There was a problem hiding this comment.
Sure. Done. Sorry for the delay
There was a problem hiding this comment.
No rush 😄 I'll look into seeing how I can de-duplicate the "missing whitespace" errors from above if you'd like. I'd prefer to avoid having the same error output twice, but with your added test case I see the extra value in the rule now - where it doesn't overlap with existing config!
There was a problem hiding this comment.
Thanks! Look forward to getting it merged!
There was a problem hiding this comment.
I tried removing 'check-module' in whitespace rule, but it did not help. It seems like they overlap indeed, but not completely. object-curly-spacing rule does not have that much flexibility. I suggest we merge it as is.
There was a problem hiding this comment.
I haven't committed to removing it yet, just trying to figure out if I could. When I removed it, it seemed like the rules directly overlapped with each other. When you checked, did you notice why it didn't help? It may have been if you checked line/column numbers only - this rule is -1 off the previous whitespace warnings since it warned you to "add after }".
There was a problem hiding this comment.
Oh, sorry. I figured you removed it there for some reason. I checked it again. It seems you're right. Object-curly-spacing covers for whitespace check-module.
There was a problem hiding this comment.
Great, I'll remove it. Just wanted to make sure we were on the same page 😄
There was a problem hiding this comment.
Is there gonna be a release any time soon? Anxious to remove object-curly-spacing rule from my tslint.json
There was a problem hiding this comment.
The release would need to be a major release so I’d like to see if anything else has changed before I do that (maybe there’s other new rules to be added). However, I’m doing less open source than I used to, so that may mean a couple of weeks time unless someone else does it.
No description provided.