[MPlayer-users] [PATCH] make cache2.c readable

[Rich Felker]

On Tue, Aug 22, 2006 at 05:38:54PM +0200, Denis Vlasenko wrote:
> Please do not remove my address from To/CC when replying.
> 
> On Tuesday 22 August 2006 05:19, Rich Felker wrote:
> > On Mon, Aug 21, 2006 at 08:18:24PM +0200, Denis Vlasenko wrote:
> > > Patch makes indentation consistent and adds
> > > some whitespace so that code is easier to read.
> > > Some long lines wrapped.
> > > 
> > > No real code changes.
> > > 
> > > Diffed against current svn. Please apply.
> > 
> > Most of these changes have nothing to do with unreadable indention or
> > spacing, just changing things to meet your own preferences for
> > indention size and whatnot.
> 
> I can format source to whatever indentation step you prefer,
> just tell me what you like more. 2? 4? 8? 3?

Leaving it however it is unless the indention levels are blatently
wrong (i.e. inner level indented less than outer one..).

> > As such I don't think it's likely for them 
> > to be applied.. If we applied a patch everytime someone had a
> > different opinion about which indention style is best we'd have 100
> > commits per day...
> 
> Sure. However, lines like this:
> 
> if(s->read_filepos<s->min_filepos) mp_msg(MSGT_CACHE,MSGL_ERR,"Ehh. s->read_filepos<s->min_filepos !!! Report bug...\n");
> 
> or this:
> 
>       if(s->stream->type!=STREAMTYPE_STREAM ||
>           read<s->min_filepos || read>=s->max_filepos+s->seek_limit)
> 
> are really hard to read because (a) '->', '>' and '>='
> look very similar without whitespace and (b) long lines
> simply scroll off out of the window in my editor.
> There should be some agreed-upon limit on line length.

Yes, adding some spaces around operators when they look confusing
without spaces would be welcome I think. Just don't do it
indiscriminately for "consistency". Yes in the cases you describe it's
bad without space, but "for (i=0; i<10; i++)" looks MUCH more readable
than "for (i = 0; i < 10; i++)" IMNSHO.

> Can you describe which changes do you find acceptable?

The purpose of whitespace is not to reflect arbitrary magical rules
about how code "should look". If this were the case then we could just
omit all whitespace entirely and let our editors choose the formatting
for us. Rather the purpose of whitespace is to make code readable.

Sometimes adding more spaces makes code more readable and other times
it makes it less readable. Often different readers will have different
opinions about which way is more readable. What I would suggest is:

- Don't mess with the spacing in someone else's code unless you're
  going to be the primary maintainer in the future or unless you
  ask them about it first.
- Only change spacing where it's particularly confusing and
  unreadable, like the above examples you gave.

Rich