-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Makefile
and .json
to editorconfig
#30324
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would decline this PR, I don’t see that it solves a problem.
[Makefile{,.pre.in,.pre}] | ||
indent_size = 4 | ||
indent_style = tab | ||
trim_trailing_whitespace = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would question an editor that does not properly handle makefiles!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's something we cannot control 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, this is harmless to have, why not add it.
trim_trailing_whitespace = true | ||
insert_final_newline = true | ||
indent_style = space | ||
|
||
[*.{py,c,cpp,h}] | ||
[*.{py,c,cpp,h,json}] | ||
indent_size = 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting JSON files does not seem necessary for this repo. I have only found a few, and they seem to be imported from external sources or maintained without issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 11 json files in the repo: https://github.com/python/cpython/search?q=language%3Ajson
I don't think json
support is any different from yml
, which we already have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know, that’s what I called a few, and you’re ignoring my second note that they are mostly copied from external sources.
I am -1 on this change. It does not solve any problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, not meant to ignore your words at all! 🙂
It was more like a personal research, because I know that are not-so-many json
files, but I was not sure about the exact amount.
I also agree that this is not a real "problem" at all, no one was ever hurt by the wrong number of spaces in a json
file (at least I really hope so!).
But, the initial reason I've created this PR was this file in its current form: https://github.com/python/cpython/blob/main/.github/problem-matchers/msvc.json
I was reading through it and asked myself: why does it have mixed formatting? What is the rule? I looked at .editorconfig
, but didn't find any. And since I was also fixing .editorconfig
files in other python/*
repositories, I've decided to create a PR that can potentially solve this tiny problem in the future.
But, I don't have strong feelings about a so-little-change, so I / you / anyone-else can totally close this PR if needed 👍
Thanks a lot for your feedback and review, TIL about tabstop
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With an old, stable codebase like CPython, the team prefers to solve real problems (reported by end users or core developers) and avoid cosmetic changes that don’t provide a concrete improvement, may introduce errors and muddle history (git log is not very good, not to mention github history that doesn’t even follow renames). Consistency in itself and for itself is not a goal! Thanks for your efforts, they will be very appreciated for other kinds of PRs!
Co-authored-by: Éric <merwok@netwok.org>
The changes for JSON are not wanted, and the remaining changes for makefiles seem too small to warrant a commit. |
There are quite a lot of
json
files that were missing from this definition.I've also added all
Makefile
s to the spec, because it is important to usetab
s when editing them.Docs: https://editorconfig.org/
Refs #27638
Refs https://bugs.python.org/issue44854
CC @ambv