Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:122423
Precedence: bulk
MIME-Version: 1.0
References: <CAK7R4ay5kE-PGkb2qn8xvjsY0nLfMGcGBTztSeAy_9=X_BwmZg@mail.gmail.com>
 <p2eUiHGGPvgqcEwgk9ISftxabO5Q5sxW6-Eoz-LrbRQBGAPorVnFPKgFaAMeLlfzVQ7Ikwr9vWDa7ctd8t9q_2spBvb2kLW-SyW869R2zQA=@gpb.moe>
 <CAPyj-LA8eDWBGqYcw7S0DTRZXN1=6HFOncwgki-ny7xYA9FnVw@mail.gmail.com> <4EB2FF2E-84F9-4603-BEDC-7A359539FD97@php.net>
In-Reply-To: <4EB2FF2E-84F9-4603-BEDC-7A359539FD97@php.net>
Date: Sun, 18 Feb 2024 05:24:09 +0100
Message-ID: <CAJmy8YGGqEsovRjQoJgwFsjr3ndsni285opNoOFdxLHshLffXQ@mail.gmail.com>
Subject: Re: [PHP-DEV] automatic formatting checks for pull requests?
To: Derick Rethans <derick@php.net>
Cc: internals@lists.php.net
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
From: divinity76@gmail.com (Hans Henrik Bergan)

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 <derick@php.net> wrote:
>
> On 17 February 2024 22:18:05 GMT, Ilija Tovilo <tovilo.ilija@gmail.com> w=
rote:
> >Hi Hans
> >
> >On Sat, Feb 17, 2024 at 3:31=E2=80=AFPM Gina P. Banyard <internals@gpb.m=
oe> wrote:
> >>
> >> On Saturday, 17 February 2024 at 11:24, Hans Henrik Bergan <hans@lolte=
k.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 codeba=
se 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 w=
orth.
> >
> >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 read=
able.
>
> 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 read=
able.

it is far from "untunable", the manpage for tuning is huge:
https://clang.llvm.org/docs/ClangFormatStyleOptions.html