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

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 3111 bytes --]

Hi Lynne

On Fri, Feb 28, 2025 at 01:24:46PM +0100, Lynne 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@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.

ok, will apply with this
this will make patcheck and developer.texi consistent.

If people want to change it, it should be changed in both locations

thx


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".