Coding style: Add .editorconfig for cross-language standardized indentation and line endings #2585
Conversation
This is a new config file which is supported by multiple editors and IDEs in order to get started with a consistent style, e.g. regarding indentations. This makes sense for both file types which are covered by linters (.cpp, .sh) as it provides a sane starting point and has additional features (e.g. config for line endings) as well as for file types which do not have linting at all. Having such a generic config file seems preferable to having none or multiple editor-specific configs.
| indent_style = space | ||
| indent_size = 2 | ||
|
|
||
| [libs/**] |
There was a problem hiding this comment.
There are no other parts which should be excluded? Are you sure (docs/)
There was a problem hiding this comment.
Hm, I don't see any obvious mistakes when editing a .md file from docs/ with vim (and the editorconfig plugin) with this config. I think the line ending/trailing LF/stripping trailing whitespace should apply as well (and works in my tests).
We should keep in mind that this is not about re-formatting existing files, but about keeping the style when creating new files or adding existing files (well, the counter example is shfmt which does use this file to decide about the proper formatting for .sh files, see #2582).
There was a problem hiding this comment.
Correct that this one is important when creating new files, but just as important, maybe even more important, is the gitconfig file.
See my latest comment on this issue.
Without the proper gitconfig, on Windows all text files are converted to CR/LF on checkout.
There was a problem hiding this comment.
I think you should check the config for ps1 files. they are typical windows and might expect cr/lf (especially the older Powershells).
There was a problem hiding this comment.
Correct that this one is important when creating new files, but just as important, maybe even more important, is the gitconfig file.
That's right, but .git/config cannot be part of a repository AFAIK, so while this is related to this PR, it's not something we can do as part of the repo.
I think you should check the config for ps1 files. they are typical windows and might expect cr/lf (especially the older Powershells).
They are already supposed to be LF, but might be converted to CRLF on Windows checkouts.
$ file .github/autobuild/windows.ps1
.github/autobuild/windows.ps1: ASCII text
$ hexdump -c .github/autobuild/windows.ps1 | head -n4
0000000 # S t e p s f o r g e n e
0000010 r a t i n g W i n d o w s a
0000020 r t i f a c t s v i a G i t
0000030 h u b A c t i o n s \n # S e
$ git show master:.github/autobuild/windows.ps1 | hexdump -c | head -n4
0000000 # S t e p s f o r g e n e
0000010 r a t i n g W i n d o w s a
0000020 r t i f a c t s v i a G i t
0000030 h u b A c t i o n s \n # S e
$ git ls-files .github/autobuild/windows.ps1 windows/* --eol
i/lf w/lf attr/ .github/autobuild/windows.ps1
i/lf w/lf attr/ windows/NSISCopyRegistryKey/README.txt
i/lf w/lf attr/ windows/NSISCopyRegistryKey/registry.nsh
i/lf w/lf attr/ windows/deploy_windows.ps1
i/-text w/-text attr/ windows/installer-banner.bmp
i/-text w/-text attr/ windows/installer-welcome.bmp
i/lf w/lf attr/ windows/installer.nsi
i/-text w/-text attr/ windows/jamulus-server-icon-2020.ico
i/-text w/-text attr/ windows/mainicon.ico
i/lf w/lf attr/ windows/mainicon.rc
i/lf w/lf attr/ windows/nsProcess/COPYING
i/lf w/lf attr/ windows/nsProcess/ConvFunc.h
i/lf w/lf attr/ windows/nsProcess/README_JamulusChanges.txt
i/lf w/lf attr/ windows/nsProcess/Readme.txt
i/lf w/lf attr/ windows/nsProcess/api.h
i/lf w/lf attr/ windows/nsProcess/nsProcess.c
i/lf w/lf attr/ windows/nsProcess/nsProcess.sln
i/lf w/lf attr/ windows/nsProcess/nsProcess.vcxproj
i/lf w/lf attr/ windows/nsProcess/nsProcess.vcxproj.filters
i/lf w/lf attr/ windows/nsProcess/nsis_tchar.h
i/lf w/lf attr/ windows/nsProcess/pluginapi.c
i/lf w/lf attr/ windows/nsProcess/pluginapi.h
i/lf w/lf attr/ windows/sound.cpp
i/lf w/lf attr/ windows/sound.h
There was a problem hiding this comment.
That's right, but .git/config cannot be part of a repository AFAIK
That's right. But you can include a .gitconfig file in the root of the repo and let contributers execute git config --local include.path ../.gitconfig to include that one in .git/config.
EDIT: Apparently you also can use .gitattributes to do the same: See here or here.
There was a problem hiding this comment.
That's right. But you can include a .gitconfig file in the root of the repo and let contributers execute
git config --local include.path ../.gitconfigto include that one in .git/config.
Yes, sure, but it defeats the reasoning for Git not supporting that by default. :)
EDIT: Apparently you also can use .gitattributes to do the same: See here or here.
Ah, I wasn't aware that those could be part of the repo. This would definitely make sense. Thanks for the pointers!
|
@pgScorpio I mean if it works with .gitattributes too, I don't think there's any harm. I'd however not see this as a replacement. |
Indeed not a replacement, but an addition. |
|
I think we should have I'll merge this PR in some days even if it lacks a second review until now. |
Short description of changes
This adds a new config file which is supported by multiple editors and IDEs in order to get started with a consistent style, e.g. regarding indentations.
This makes sense for both file types which are covered by linters (.cpp, .sh) as it provides a sane starting point and has additional features (e.g. config for line endings) as well as for file types which do not have linting at all.
Having such a generic config file seems preferable to having none or multiple editor-specific configs.
See:
https://editorconfig.org
CHANGELOG: Coding Style: Added .editorconfig for cross-language standardized indentation and line endings
Context: Fixes an issue?
Related: #2552 (cc @pgScorpio)
Does this change need documentation? What needs to be documented and how?
Having the file is useful on its own as many tools detect it automatically.
In addition, we could add it to CONTRIBUTING.mdin order to point users to install editor/IDE plugins, but I'm not sure if it's worth it (cc @ann0see).
Status of this Pull Request
Ready
What is missing until this pull request can be merged?
Reviews
Checklist