i have tested running clang-format against the entire php-src codebase, and there is only 1 file it breaks: ext/spl/spl_directory_arginfo.h more details at https://github.com/php/php-src/pull/13417#issuecomment-1950920114
On Sun, 18 Feb 2024 at 01:45, Derick Rethans <der...@php.net> wrote: > > On 17 February 2024 22:18:05 GMT, Ilija Tovilo <tovilo.il...@gmail.com> wrote: > >Hi Hans > > > >On Sat, Feb 17, 2024 at 3:31 PM Gina P. Banyard <intern...@gpb.moe> wrote: > >> > >> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan > >> <h...@loltek.net> wrote: > >> > >> > Can we add automatic formatting checks for pull requests? > >> > Made a PR: https://github.com/php/php-src/pull/13417 > >> > >> It would be nice to have some formatting rules to harmonize the codebase > >> as it is somewhat the wild west, > >> but as far as my understanding goes is that Clang format struggles to > >> understand our codebase (namely macros) and is difficult to set-up for > >> php-src. > > > >Right. Consistent code style is nice, but what we have now is really > >not that bad. There are a couple things I'd want if we enforce code > >style: > > > >* Fixing the style should be easy, running a single command without > >first pushing to CI. > >* It should be fast too, so that I can easily run it for every commit, > >preferably even on-save in my editor. > >* The new code style should be applied only to newly added sections or > >changed code, not entire files. Otherwise, we'll have many changes in > >large files, with endless merge conflicts when merging up from lower > >branches. > >* The formatting tool should work for all php-src code, not just plain > >C code. We don't want to be forced to refactor old macros just because > >we need to add a single line to some long-standing code. Last time I > >tried clang-format, it utterly failed with our macros. > > > >I haven't looked at your PR in detail, so I'm not sure which of these > >points it satisfies. It would be great if you could quickly describe > >how it works, and what the goals are. > > > >Essentially, I'm just sceptical that this isn't more trouble than it's worth. > > > >Ilija > > IMO, clang-format isn't really suitable. Its untunable style is often far > from the coding style that we currently have, and it makes some really odd > choices as to when and how to wrap lines, making code definitely less > readable. > > cheers > Derick > IMO, clang-format isn't really suitable. Its untunable style is often far > from the coding style that we currently have, and it makes some really odd > choices as to when and how to wrap lines, making code definitely less > readable. it is far from "untunable", the manpage for tuning is huge: https://clang.llvm.org/docs/ClangFormatStyleOptions.html