coding style: fixed indenting vs align-with-brace

[Neels Hofmeyr]

Recently, fixeria brought up that it would be better to not align wrapped lines
with an opening brace, but rather have a constant indent for wrapped lines.

I'd like to ask for the general opinion / kernel style compliance.
What indenting is acceptable?

For a long time my opinion was that opening brace alignment is better for
readability, but my opinion has shifted. I now find it more desirable to keep a
constant indent.

Consider:

unsigned int my_function_signature(int a_very_long_parameter,
				   int wrapped_line,
				   int another_line);

If a patch modifies the return value or the name, the patch either touches
three lines or leaves inconsistent indenting behind:

int my_func(int a_very_long_parameter,
				   int wrapped_line,
				   int another_line);

Also, a very long function name wastes a lot of usable linewidth for indent for
all arguments.

I've submitted a lot of patches over the years that touch more lines than
necessary, and left quite a few inconsistent indents behind, because an
automatic find-replace doesn't fix subsequent indenting.

I'm currently editing a lot of the osmo-msc sources and changing my mind about
API, so this line indenting due to renames issue comes up. I thought I could
pick a constant indent now while I'm at it.

If we had a single tab as indent, the function signatures' indenting would be
independent from the length of the name, and a find-replace leaves behind
consistent indenting / no whitespace changes needed for renames:

unsigned int my_function_signature(int a_very_long_parameter,
	int wrapped_line,
	int another_line);

int my_func(int a_very_long_parameter,
	int wrapped_line,
	int another_line);

I finally understood the vim cindent options to achieve this:

	:se cino=(1s,u1s
	
(Forgive me if this is a bit too vim specific for your taste; I definitely want
vim to automatically give me the desired indenting.)

My problem with that is that it also affects if- and for- statements: 'if' and
'for' will never change their name, so aligning with opening-brace is unlikely
to ever cause unnecessary whitespace change. What was:

// vim: cino=>1s,e0,f0,{0,}0,=1s,t0,c3,+1s,(0,u0,/0,:0

int my_func(int a_very_long_parameter,
	    int wrapped_line,
	    int another_line)
{
	if (a_very_long_parameter == 3 || a_very_long_parameter < 0
	    || wrapped_line == 7
	    || (one_level_deeper
		&& (one_level_deeper
		    || foo)))
		return 5;
}

Now becomes:

// vim: cino=>1s,e0,f0,{0,}0,=1s,t0,c3,+1s,(1s,u1s,/0,:0

int my_func(int a_very_long_parameter,
	int wrapped_line,
	int another_line)
{
	if (a_very_long_parameter == 3 || a_very_long_parameter < 0
		|| wrapped_line == 7
		|| (one_level_deeper
			&& (one_level_deeper
				|| foo)))
		return 5;
}

(The cino=k0 parameter does control 'if', 'for' and 'while' separately, but
there is no way to tell it to brace-align while the '(' option is set to
'(1s', stupid quirk in cinoptions.)

IMHO aligning with opening brace improves readability of the nested conditions
and uses more of the line width. But I could get used to that, I guess. I think
I've seen this used a lot in Osmocom, too.

Another thing to consider:

	if (a_very_long_parameter == 3 || a_very_long_parameter < 0
		|| wrapped_line == 7)
		return has_same_indent_as_condition(" :/ ");

I could use two tabs for in-brace indenting, but I haven't seen that anywhere
before, and it makes wrapped conditions waste a lot of line width:

// vim: cino=>1s,e0,f0,{0,}0,=1s,t0,c3,+1s,(2s,u1s,/0,:0

int my_function_signature(int a_very_long_parameter,
		int wrapped_line,
		int another_line)
{
	if (a_very_long_parameter == 3 || a_very_long_parameter < 0
			|| wrapped_line == 7
			|| (one_level_deeper
				&& (one_level_deeper
					|| foo)))
		return 5;

	if (a_very_long_parameter == 3 || a_very_long_parameter < 0
			|| wrapped_line == 7)
		return 5;
}


I think I'll adopt fixed single-tab indenting in my patches from now on:
:se cino=>1s,e0,f0,{0,}0,=1s,t0,c3,+1s,(1s,u1s,k0,/0,:0

Any (strong) opinions on / best practices for this?
Which one is more consistent with linux kernel style?

Thanks!

~N
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20190206/5a6c5ff1/attachment.bin>