[MPlayer-dev-eng] [PATCH] libvo: GGI driver update

[Christoph Egger]

> Hi,
> On Tue, Sep 06, 2005 at 01:34:58PM +0200, Christoph Egger wrote:
> > > The version which is currently in CVS mixes tabs and spaces.
> > > This patch switches to tabs only (and overall codingstyle to K&R)
> 
> I personally prefer spaces only, since they always have the same size,
> and I also think you would have to do less changes.
>
> > I'll attach the patch with the cosmetic changes only again.
> > Just to be sure you look into the _right_ patch.
> 
> So the switch and case statements are supposed to be on the same level?

Yes. What you see is the switch to K&R C-style. Linux, *BSD
and many other projects use it. It's a quasi-standard.

> line 77 from the patch
> +	switch(format) {
>  	case IMGFMT_RGB4:
> Well, not my style..

I can live that statement.
What is the mplayer's _general_ coding style?

> And there still are space used e.g. here (line 398):
>  #if 1
> -       (IMGFMT_IS_RGB(mpi->imgfmt) &&
> +           (IMGFMT_IS_RGB(mpi->imgfmt) &&
>             (IMGFMT_RGB_DEPTH(mpi->imgfmt) != vo_dbpp)) ||
> -       (IMGFMT_IS_BGR(mpi->imgfmt) &&
> +           (IMGFMT_IS_BGR(mpi->imgfmt) &&
>             (IMGFMT_BGR_DEPTH(mpi->imgfmt) != vo_dbpp)) ||
> 
> I can't believe I'm reviewing a cosmetics patch, and to put it bluntly,

As I told above, this is K&R coding style.
The 2nd patch with the functional changes replaces this #if 1...#endif
block anyway. I'll send the 2nd patch once the cosmetic changes are
in CVS.

> I don't know if you'll actually find someone applying this

You give me the impression the quality/readability of the patch counts
rather the result after applying the patch...

Tell me if I'm wrong...

> (unless you get CVS access yourself).

I'm told be the maintainer for the ggi driver (However, I'm not aware
of any account information). The code in CVS (up to rev 1.37) is a mix
of tabs and spaces. I believe this state comes over time from different
people, one likes tabs, the other likes spaces...
As a maintainer I should have the right to
say 'Stop. Please don't make it worse...'

Again tell me, if I am wrong...

-- 
Greetings,

Christoph

5 GB Mailbox, 50 FreeSMS http://www.gmx.net/de/go/promail
+++ GMX - die erste Adresse für Mail, Message, More +++