Conversation
See https://editorconfig.org/ And .gitattributes for consistency (ensures line endings are not transformed when working on the same folder from different systems, e.g. WSL / Windows)
.editorconfig
Outdated
| # https://editorconfig.org | ||
|
|
||
| [*] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| insert_final_newline = true | ||
| trim_trailing_whitespace = true | ||
|
|
||
| [*.css] | ||
| indent_style = tab | ||
|
|
||
| [*.js] | ||
| indent_style = tab | ||
|
|
||
| [*.json] | ||
| indent_style = tab | ||
|
|
||
| [*.{markdown,md}] | ||
| indent_size = 4 | ||
| indent_style = space | ||
|
|
||
| [*.sql] | ||
| indent_style = tab | ||
|
|
||
| [*.yml] | ||
| indent_size = 2 | ||
| indent_style = space |
There was a problem hiding this comment.
I would recommend a couple of improvements:
This is the top-level definition, so let's mark it as root.
root = true
You can combine these for efficiency:
[*.{css,html,js,json,sql}]
indent_style = tab
Use both standard file extensions:
[*.{yaml,yml}]
indent_size = 2
indent_style = space
Markdown has some additional rules that are important:
[*.{markdown,md}]
indent_size = 4
indent_style = space
max_line_length = 0
trim_trailing_whitespace = false
Merge these in, and I'm happy to approve.
There was a problem hiding this comment.
I will do as you ask, but here is nevertheless some rationale for the current choices:
root = trueprevents users from choosing the display size of their tab indentations (the idea by not having this rule being that users can have an even higher.editorconfigwith cosmetic preferences)[*.{css,html,js,json,sql}]fine but makes it more difficult to have distinct rules per language[*.{yaml,yml}]there is no*.yamlfile in this projectmax_line_lengthdoes not exist, does it?trim_trailing_whitespace = falsealthough possible to want trailing whitespace in Markdown, it is not recommended anymore as far as I know, and I do not think SimplePie makes conscious use of it. e.g. https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md#md009
There was a problem hiding this comment.
root = trueprevents users from choosing the display size of their tab indentations (the idea by not having this rule being that users can have an even higher.editorconfigwith cosmetic preferences)
Ah. I thought we'd gotten past this by now. PSR-1 and PSR-2 have been out for about a decade, and I'm just now realizing that SimplePie never adopted them. OK. We can remove this.
[*.{css,html,js,json,sql}]fine but makes it more difficult to have distinct rules per language
You would only collapse them as it makes sense. You expand if it makes sense. In the case of these rules, this is the most collapsed form which makes sense.
[*.{yaml,yml}]there is no*.yamlfile in this project
Sure. This isn't a big deal, it just covers both common file extensions.
max_line_lengthdoes not exist, does it?
It's supported on EditorConfig.org. And at one point it was part of the official SimplePie coding style guide that the line length was 120 characters. But I haven't been a code contributor on this project since 2010. So YMMV.
trim_trailing_whitespace = falsealthough possible to want trailing whitespace in Markdown, it is not recommended anymore as far as I know, and I do not think SimplePie makes conscious use of it. e.g. https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md#md009
I would say it should definitely be enabled for everything except for Markdown.
As for Markdown specifically, you can mark a stanza linebreak with either two spaces ( ), or a space followed by a backslash ( \). This is a very specific usage that isn't super-common (i.e., stanzas). But we don't want to break it when it's necessary.
If we agree to use the latter format ( \) instead of the former format ( ), then we can enable this one as well.
There was a problem hiding this comment.
I have just removed the root constraint as agreed 8d6af85
Regarding whitespace for line breaks, a search shows that this is not something used in the current SimplePie Markdown files. There are five instances of single trailing spaces in https://github.com/simplepie/simplepie/blob/master/CHANGELOG.md but those are clearly mistakes. I would vote for the backslash syntax \ or explicit <br />.
Fix regression from simplepie#738 See simplepie#724 Sad to see all these wasted / inefficient / hard-coded spaces though... Tabs ❤️
See https://editorconfig.org/
Helps text editors automatically adopt the conventions (e.g. for white-space) used in this specific project.
And
.gitattributesfor consistency: ensures line endings are not transformed when working on the same folder from different systems, e.g. WSL / Windows