[FFmpeg-devel] doc/developper: always use braces for statements

---
 doc/developer.texi | 8 ++++++++
 1 file changed, 8 insertions(+)

Nicolas Gaullier (12019-08-06):
> ---
>  doc/developer.texi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index 5c342c9106..1e95768364 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -213,6 +213,14 @@ please use av_log() instead.
>  @item
>  Casts should be used only when necessary. Unneeded parentheses
>  should also be avoided if they don't make the code easier to understand.
> +
> +@item
> +Single expression statements after if or while should always use braces:

No they should not. What makes you think that?

> +@example
> +    if (x) @{
> +        return;
> +    @}
> +@end example
>  @end itemize
>  
>  @section Editor configuration

Regards,

Aug 6, 2019, 5:33 PM by george@nsup.org:

> Nicolas Gaullier (12019-08-06):
>
>> ---
>>  doc/developer.texi | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/doc/developer.texi b/doc/developer.texi
>> index 5c342c9106..1e95768364 100644
>> --- a/doc/developer.texi
>> +++ b/doc/developer.texi
>> @@ -213,6 +213,14 @@ please use av_log() instead.
>>  @item
>>  Casts should be used only when necessary. Unneeded parentheses
>>  should also be avoided if they don't make the code easier to understand.
>> +
>> +@item
>> +Single expression statements after if or while should always use braces:
>>
>
> No they should not. What makes you think that?
>

+1

tis 2019-08-06 klockan 18:33 +0200 skrev Nicolas George:
> Nicolas Gaullier (12019-08-06):
> > ---
> >  doc/developer.texi | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/doc/developer.texi b/doc/developer.texi
> > index 5c342c9106..1e95768364 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -213,6 +213,14 @@ please use av_log() instead.
> >  @item
> >  Casts should be used only when necessary. Unneeded parentheses
> >  should also be avoided if they don't make the code easier to
> > understand.
> > +
> > +@item
> > +Single expression statements after if or while should always use
> > braces:
> 
> No they should not. What makes you think that?

I think they should. See 
https://www.imperialviolet.org/2014/02/22/applebug.html

/Tomas

Tomas Härdin (12019-08-06):
> I think they should. See 

Documenting the coding standard and changing it are different matters.

> https://www.imperialviolet.org/2014/02/22/applebug.html

test_warn.c: In function ‘test’:
test_warn.c:6:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
     if (x == y)
     ^~
test_warn.c:8:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
         return -1;
         ^~~~~~

Regards,

On 2019-08-06 22:51 +0200, Nicolas George wrote:
> Tomas Härdin (12019-08-06):
> > I think they should. See
>
> Documenting the coding standard and changing it are different matters.

I guess a change was intended, because it should be fairly obvious
that in the current code base the described style for ifs with a single
statement body is barely used.

IMHO the only con is that it uses up more vertical space (one line
more, when sticking with the brace placement as is) and thus less real
code fits on a screen/page.

Anyway despite of other things pointed out below. There are additional
advantages like making it easier to debug and to extend the code with
less changes. In turn making the potentially resulting patches shorter
and easier to read.


> > https://www.imperialviolet.org/2014/02/22/applebug.html
>
> test_warn.c: In function ‘test’:
> test_warn.c:6:5: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation]
>      if (x == y)
>      ^~
> test_warn.c:8:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’
>          return -1;
>          ^~~~~~


After all Style is subjective, and thus of course you are right,
Nicolas, changes to the coding style should be discussed and not
be tunneled through documentation patches. Though I don't think
the latter was the intention of the OP. Clearly communicating the
request for change would have been better though.


  Alexander

On Wed, Aug 07, 2019 at 00:14:06 +0200, Alexander Strasser wrote:
> Nicolas, changes to the coding style should be discussed and not
> be tunneled through documentation patches. Though I don't think
> the latter was the intention of the OP.

It was probably caused by different reviews:
- "Please remove the brackets."
- "Please add brackets."

I don't feel strongly about style rules being changed (though I do
prefer one of the two), but it should be consistent and documented.

(Same e.g. about the loop counter declaration within for(;;).)

Cheers,
Moritz

Moritz Barsnick (12019-08-07):
> On Wed, Aug 07, 2019 at 00:14:06 +0200, Alexander Strasser wrote:
> > Nicolas, changes to the coding style should be discussed and not
> > be tunneled through documentation patches. Though I don't think
> > the latter was the intention of the OP.
> 
> It was probably caused by different reviews:
> - "Please remove the brackets."
> - "Please add brackets."
> 
> I don't feel strongly about style rules being changed (though I do
> prefer one of the two), but it should be consistent and documented.

I think it is one of the cases that are best left to the discretion of
whoever writes the code, or whoever maintains it if they have a strong
opinion about it: there are more case-by-case factors than general
arguments.

And the communication about it was clumsy, but let us not dwell on it.

Regards,