Add editorconfig #1102
Add editorconfig #1102
Conversation
|
I don't think it's a good idea to include this file unless we can validate it against our existing code. Do you know of a tool that can do that? |
|
Well, there is https://github.com/jedmao/eclint. I'm unable to make it work properly, though, and it seems unmaintained... But the files use 2 space indent, trailing newline, and no trailing spaces. All of these are validated by eslint already (maybe not utf-8, but that's usually default in modern editors, and I doubt this repo uses anything other than ascii either way), this file just automatically sets the IDE/text editor to those same settings. |
|
This may be what you need, I haven't looked deeply into it but it seems good. |
|
@Vadorequest so the test would be to run the tool, and expect |
|
I think I misunderstood what you wanted, I was looking for a tool to generate a .editorconfig based on ESLint. But yeah, you could generate the file using this module and match it against yours. |
|
@ljharb Found a tool to run a check on CI (same that webpack uses) |
| max_line_length = null | ||
|
|
||
| [*.md] | ||
| trim_trailing_whitespace = false |
There was a problem hiding this comment.
why would we not want to trim trailing whitespace on markdown files?
There was a problem hiding this comment.
MD has significant whitespace, so if you want e.g. a linebreak without starting a new paragraph, you add two trailing spaces.
Want me to remove it?
There was a problem hiding this comment.
Interesting, I was not aware of that.
It's fine to leave as-is then.
| @@ -4,6 +4,7 @@ | |||
| "description": "Airbnb's base JS ESLint config, following our styleguide", | |||
| "main": "index.js", | |||
| "scripts": { | |||
| "prelint": "editorconfig-tools check *.js rules/*.js test/*.js", | |||
There was a problem hiding this comment.
huh, i wasn't aware you could "pre" any old script. Seems this works all the way back to npm 1.
There was a problem hiding this comment.
This should probably cover jsx files too.
| @@ -0,0 +1,14 @@ | |||
| root = true | |||
|
|
|||
| [*] | |||
There was a problem hiding this comment.
if this is *, then the editorconfig check should also use * - if it's just js/jsx, then i'd like both places to match.
|
Changed to run against every file |
|
Yes, it walks up the tree. I know it worked because it gave me failures on too long lines before I disabled that rule |
|
I don't think the file needs to be included in a publish, it's just useful for contributors, having their IDE configured automatically |
|
@SimenB |
|
hm, ok. add a symbolic link, duplicate it, or copy it over during pubilsh (and gitignore it)? |
|
I think the symlink is probably the simplest solution. |
|
Ok, symlinks pushed |
|
@ljharb rebased |
|
hmm - when i try this locally, even if i add a hard tab, it doesn't error out. |
|
Hmm, it doesn't seem to be able to check indent style... In the source code (https://github.com/slang800/editorconfig-tools/tree/master/lib/rules) it's called It does correctly yell at me for trailing spaces, trailing newline, CRLF and size of indentation, though. And it also yelled at me for line length, but as it\s unable to ignore long strings or comments, it gave a lot of false positives. A bummer it doesn't seems to check type of indent, but extra indent gets yelled at |
|
Also, setting |
|
gotcha - seems i just got lucky and picked the one thing it can't detect. oh well, this seems fine :-) |
|
Hmm, If I set all indentation to tabs, it yells at me for not using spaces... Seems like a bug |
|
@SimenB please file that upstream if you don't mind ;-) |
|
Done, notslang/editorconfig-tools#36 Thanks for merging! |
It silences IntelliJ a least 😄
I think this should match the style in the repo.