Re: [fluid-dev] More commits

[josh]

Quoting David Henningsson <address@hidden>:
> address@hidden skrev:
> > Anyone else have any opinions on the matter?
>
> Of course I have, but it seems like I will be voted down in most of the
> cases below :-)


I'm hoping we can find a happy medium, where the most annoying  
formatting to any one of us is not used.  One or more of us may still  
have to live with some minor annoyances though, but I doubt that will  
lead to permanent insanity ;-)


> - Spaces per identation level: don't care much about the exact amount,
> and skipping tabs altogether is a good idea.


I like 2 or 4 spaces.  So it sounds like 4 spaces it is (Pedro's  
preference) with no tabs.  I imagine subversion can be setup to  
automatically convert tabs to spaces, that would be nice for checkins.


> I don't like to indent to
> the amount of a procedure name,
> because_if_you_have_a_very_long_function_name(that,
>                                               will,
>                                               mean,
>                                               that,
>                                               you,
>
>                                               will,
>                                               have,
>                                               to,
>                                               put,
>                                               every,
>                                               parameter,
>                                               on, a,
>                                               new line);


In cases like that, I often cheat and just do:

  because_if_you_have_a_very_long_function_name
    (that, will, mean, that, you, ...);

I personally like following lines to line up with the parenthesis they  
belong to, makes it easier to group the statements visually and know  
which set of parenthesis they belong to.

> - Prefer having opening braces on same line (except for function bodies).


It seems both you and Pedro like this and its only a minor annoyance  
to me, so I think this sounds fine.  Its how current FluidSynth source  
is anyways (for the most part).


> - Extra spaces before and after parentheses, don't like it.


It seems none of us agree on parenthesis at the moment..


> - Extra row just for the function return type, don't like it.


The advantage of this is that you can do a grep for ^function_name and  
easily find where it is actually defined, versus where it might be  
used.  For me it makes it much easier to spot the function name  
immediately too, rather than having to scan over the return type.  It  
seems Pedro likes it this way too, so you may have to live with it ;)


> I guess my motivation for "compacting" the code the less lines you use,
> and the more code you can read without scrolling, which gives a better
> overview.
>
> Btw, do we have an approximately max lines per unit?


Not sure what you mean by that.  Do you mean what is the current line  
count in FluidSynth source for the file with the most lines?


> So here's my sample (go ahead and puke if you don't like it ;-) )
>
> fluid_thread_t* new_fluid_thread(fluid_thread_func_t func, void* data,
>     int detach)
> {
>     GThread* thread;
>     GError* err = NULL;
>
>     g_return_val_if_fail(func != NULL, NULL);
>
>     thread = g_thread_create((GThreadFunc) func, data, detach == FALSE,
>         &err);
>     if (!thread) {
>         FLUID_LOG(FLUID_ERR, "Failed to create the thread: %s",
>             fluid_gerror_message(err));
>         g_clear_error(&err);
>     }
>     return thread;
> }


Ahh, that isn't so bad!  I've definitely seen worse ;)


> // David

Voting summary:
- No tabs (Pedro, Josh and David)
- 4 space indentation (Pedro, Josh 2 or 4, David doesn't care)
- Return type on separate line from function (Pedro and Josh)
- Space between type and '*' (GThread *) (Pedro and Josh)
- Braces on same line as previous statement except procedures (Pedro  
and David)
- Additional statements line up with '(' and '[' (Pedro and Josh)


Seems we don't agree on spacing before and after parenthesis.  If no  
one objects strongly, I'd like to go with my preference (space before  
'(' and after ')').  Or perhaps just keep it the way FluidSynth  
currently is.

Regards,
     Josh