hipl-core team mailing list archive

[Diego Biurrun <diego@xxxxxxxxxx>]

On Sun, Aug 29, 2010 at 04:03:50PM +0200, Christof Mroz wrote:
> On Sun, 29 Aug 2010 13:03:23 +0200, Diego Biurrun <diego@xxxxxxxxxx> wrote:
>
>>>> It should be described in the HACKING files of both PISA and HIPL.
>>>> Have you read them?
>>>
>>> Yes but obviously not thoroughly enough :) didn't notice there's a
>>> distinction between assignments and function calls regarding indention.
>>
>> How can this be made clearer?
>
> I'd propose this:
>
> --- doc/HACKING 2010-05-26 13:09:00 +0000
> +++ doc/HACKING 2010-08-29 13:51:58 +0000
> @@ -489,12 +489,14 @@
>
> +  - Function calls should not be mechanically indented by four spaces.
> +    Instead they should be indented for optimum readability. For example,
> +    arguments placed on the next line should align with the first
> +    character after the '('. If no argument fits on the current
> +    line, place the opening brace on the next indented by 8 spaces.
> +  - Cases not covered by the above rules, like assignments,
> +    should be indented by 8 spaces.

This 8 characters idea is completely arbitrary and IMO not an improvement
at all.  Creating more and arbitrary rules is not good.

Check out the following examples:

  variable_with_a_long_name = struct_with_a_long_name.member_name +
      other_struct_with_a_long_name->member_name

  variable_with_a_long_name = struct_with_a_long_name.member_name +
          other_struct_with_a_long_name->member_name

No improvement in readability, but the 8 spaces thing is something extra
you need to keep in your head.  Now compare with the following:

  variable_with_a_long_name = struct_with_a_long_name.member_name +
                              other_struct_with_a_long_name->member_name

This is an actual readability improvement because the indentation
follows the logical structure.  The second line is clearly part of
the value being assigned.

> @@ -553,6 +555,23 @@
>
> +static void blah(int a, int b) {
> +    /* excessive nesting should be avoided but if it can't,
> +     * remember the following rules */
> +    if(a) {
> +        if(b) {
> +            struct opaque blah;

*sigh* - This is not even in K&R style.  May I politely suggest that you
go back to reading its definition?  (hint: spaces, keywords)

> +            /* opening '(' on next line if no argument fits */
> +            yet_another_function_with_a_long_name
> +                    (a_function_with_a_long_name(NULL, &blah, 0xC0FFEE));

This is not K&R style and completely counterproductive.  You destroy the
visual cue that a string followed by an opening parenthesis gives you -
in K&R style you immediately know it's a function call, now you don't
anymore.

Diego