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

[Diego Biurrun]

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:
>  >
> >> diego wrote:
> >> 
> >> > Log:
> >> > Factorize print_version().
> >> > 
> >> > [...]
> >> > @@ -24,6 +28,47 @@ ass_track_t* ass_track = 0; // current t
> >> >  
> >> > +#if HAVE_MMX
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," MMX");
> >> > +#endif
> >> > +#if HAVE_MMX2
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," MMX2");
> >> > +#endif
> >> > +#if HAVE_3DNOW
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," 3DNow");
> >> > +#endif
> >> > +#if HAVE_3DNOWEX
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," 3DNowEx");
> >> > +#endif
> >> > +#if HAVE_SSE
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," SSE");
> >> > +#endif
> >> > +#if HAVE_SSE2
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V," SSE2");
> >> > +#endif
> >> > +    mp_msg(MSGT_CPLAYER,MSGL_V,"\n");
> >> > +#endif /* RUNTIME_CPUDETECT */
> >> > +#endif /* ARCH_X86 */
> >> > +}
> >> 
> >> 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.

Diego