[FFmpeg-devel] [PATCH] doc/developer: Better {} style rule

[Vittorio Giovara]

On Fri, Feb 28, 2025 at 1:25 PM Lynne <dev at lynne.ee> wrote:

>
>
> On 28/02/2025 03:33, Michael Niedermayer wrote:
> > On Fri, Feb 28, 2025 at 03:25:06AM +0100, Lynne wrote:
> >> On 27/02/2025 02:10, Michael Niedermayer wrote:
> >>> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> >>> ---
> >>>    doc/developer.texi | 11 +++++------
> >>>    1 file changed, 5 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/doc/developer.texi b/doc/developer.texi
> >>> index a1bfe180c9b..6a753f99da6 100644
> >>> --- a/doc/developer.texi
> >>> +++ b/doc/developer.texi
> >>> @@ -179,18 +179,17 @@ int fields = ilace ? 2 : 1;
> >>>    @end example
> >>>    @item
> >>> -No braces around single-line blocks:
> >>> +No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
> >>>    @example c, good
> >>>    // Good
> >>> -if (bits_pixel == 24)
> >>> +if (bits_pixel == 24) @{
> >>>        avctx->pix_fmt = AV_PIX_FMT_BGR24;
> >>> -else if (bits_pixel == 8)
> >>> +@} else if (bits_pixel == 8) @{
> >>>        avctx->pix_fmt = AV_PIX_FMT_GRAY8;
> >>> -else @{
> >>> -    av_log(avctx, AV_LOG_ERROR, "Invalid pixel format.\n");
> >>> +@} else
> >>>        return AVERROR_INVALIDDATA;
> >>> -@}
> >>> +
> >>>    @end example
> >>>    @item
> >>
> >> Strongly objecting to this patch. It's all over our codebase, and I
> like it.
> >> Literally everywhere you look.
> >> If future patches need to add more under the branch, then it only needs
> to
> >> be done once.
> >>
> >> Such fundamental changes need more than a single patch hastily
> accepted, and
> >> should involve the community as a whole, I believe.
> >
> > Its not a change, look at tools/patcheck
> >
> > or try:
> >
> > +    if(this)
> > +        that
> > +    else
> > +        nothat
> > +
> >
> > patcheck reports:
> >
> > missing } prior to else
> > patcheck.stdout:11:+    else
> >
> > missing whitespace between keyword and ( (feel free to ignore)
> > patcheck.stdout:9:+    if(this)
>
> Ah, okay. I didn't read properly. The double negative in the sentence
> makes it hard to understand.
>
> Rather than
>  > No braces around single-line blocks, unless they are followed by an
> else (to keep future patches cleaner)
>
> Consider
>  > Don't wrap single-line blocks in braces. Use braces only if there is
> an accompanying else statement. This keeps future code changes easier to
> keep track of.


I'm not sure it's a good idea, personally I've gotten used to go enforcing
brackets all the time and it definitely feels less error-prone
I think having a formatting rule that implements this, even for a C
codebase, will be beneficial in the long run
-- 
Vittorio