Message ID | 20190806162135.8728-1-nicolas.gaullier@arkena.com |
---|---|
State | New |
Headers | show |
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,
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: +@example + if (x) @{ + return; + @} +@end example @end itemize @section Editor configuration