[MPlayer-cvslog] r28360 - in trunk: Makefile mencoder.c mpcommon.c mpcommon.h mplayer.c version.sh

[The Wanderer]

Diego Biurrun wrote:

> On Sat, Feb 21, 2009 at 10:49:14PM -0500, The Wanderer wrote:
 >
>> Diego Biurrun wrote:
>> 
>>> On Sun, Jan 25, 2009 at 11:54:53PM +0100, Aurelien Jacobs wrote:

>>>> Unrelated to this commit, but this could be written nicer,
>>>> smaller, more readable:
>>>> 
>>>> if (HAVE_MMX)    mp_msg(MSGT_CPLAYER,MSGL_V," MMX"  );
>>>> if (HAVE_MMX2)   mp_msg(MSGT_CPLAYER,MSGL_V," MMX2" );
>>>> if (HAVE_3DNOW)  mp_msg(MSGT_CPLAYER,MSGL_V," 3DNow");
>>> 
>>> Done, thanks for the hint.
>> 
>> As far as I can tell, it wasn't done, at least not as suggested.
>> 
>> As I read it, the important part of the suggested change was the
>> readability increase, which in turn derives almost entirely from
>> moving the mp_msg call onto the same line as the conditional and
>> doing vertical alignment as appropriate. In order to achieve that,
>> it's necessary to convert the #if X into if(X), but that change is
>> not itself especially important.
>> 
>> Commit 28375, which I think is the one corresponding to the change
>> you're talking about, does the #if -> if() conversion but does not
>> combine the lines. I don't think I see how this is an improvement.
>> Shouldn't the rest of the suggested change be made?
> 
> No, statements on the same line as the if are an abomination.

I don't normally like them either, but in this case they would help
improve readability, and without them the change which you did make
seems IMO completely pointless. I'm not going to shout about it much,
though...

-- 
       The Wanderer

Warning: Simply because I argue an issue does not mean I agree with any
side of it.

Secrecy is the beginning of tyranny.