Junio C Hamano <gitster@xxxxxxxxx> writes: > Cc-list chosen from "git shortlog --since=12.months --no-merges .clang-format". > > I am wondering how often our developers use "make style" aka > > git clang-format --style file --diff --extensions c,h > > and also wondering if the suggested style fixes are really > "improvements". For example, taking randomly the latest patch I > just injested into my tree, i.e. > > $ git am a-single-patch-file.txt > $ git reset --soft HEAD^ > $ make style > > I got the output attached at the end of the message. The result is > a mixed bag (I commented on the "patch" as if it were a patch > submission). > > I have this suspicion that nobody complained these sub-par > suggestions the tool makes based on what we have in .clang-format > because not many folks run "make style", and "make style" is not > very easy to use after you record your changes into a commit. IOW, > there is nothing packaged to help "I have four commits on top of the > upstream, I want to run style checks before running format-patch", > i.e. > > git clang-format --diff HEAD~4 > > Even the output from the tool is of mixed quality, there are good > pieces that can be used to improve your patches. So we may prefer > to see the tool used more often, but not in a way to suggest its > output is always better than what the human developer has written. > > For that, there are a few things we'd probably need to do: > > - Improve our tooling so that the develper can check a range of > commits they made before running format-patch, and other > situations. > > - Improve .clang-format rules to reduce false positives. > I think the biggest issue for this is around line wrapping, I'm considering just removing it from the '.clang-format'. Perhaps we could add it to our '.editorconfig'? The issue itself is that we don't always wrap to 80 characters. We try to wrap to 80 characters, but prioritize readability over this limit. This is hard to do in '.clang-format' since it requires tuning the penalties, which have arbitrary values. I did try to do that in 5e9fa0f9fa (clang-format: re-adjust line break penalties, 2024-10-18), but as we see, that wasn't very successfull. If that sounds okay, I can send in a patch to do that and also making 'RemoveBracesLLVM: true' a permanent rule. [snip]
Attachment:
signature.asc
Description: PGP signature
