ok, but looking at https://review.coreboot.org/#/c/coreboot/+/26512/, is there any single thing in there that stands out as a a problem? Every single change looks better to me.
I still don't see the problem with 132. I did a quick check on the Go source tree, where there basically are no limits for 10 years now, and there are very few cases where lines are longer than 80, but in those cases, readability is superior for those long lines. I suspect, based on my little test, that the same is true of coreboot. People tend to write to 80, but occasionally, things are longer, and at least for me, this
printk(BIOS_DEBUG, "Exception: %s\n", exception_names[tf->cause]);
is much more readable than this
printk(BIOS_DEBUG, "Exception: %s\n", exception_names[tf->cause]);
I'm fine with strongly recommending we stick to 80, but I'd really like to get away from making it mandatory. And, further, I'd like to see us move to clang-format on commits, so we can end these discussions as they have ended them in the Go and Rust communities.
If somebody every submits a 512-length line, I think we can convince them to see reason :-)
ron
On Fri, May 25, 2018 at 9:41 AM Nico Huber nico.h@gmx.de wrote:
On 25.05.2018 10:15, Patrick Georgi via coreboot wrote:
That is, who would unbearably suffer from 132 characters gper line of
code?
Humans. Code quality.
Eyes get too tired too fast. The cost of our eyes' "carriage return" grows over-linear with the line length. Although, that's hard to measure, I'm sure 132 chars would have a negative impact on review quality. The cost of comparing two views side-by-side increases too, with the distance between them.
A general rule from printed text is that you should keep the column width below 70 chars on average. You can't apply that directly to fixed- width fonts and, obviously, code lines are not like running text (ide- ally a statement ends before hitting any line break). But, in every pro- ject that allows longer lines, I've seen people starting to use the extra space also for comments. And comments at 132 chars? that would really be unbearable.
IMHO, the problem with 80 chars is that it is just a little too nar- row for code. It's a good length at function scope but even only two additional levels of indentation, and you have only 56 chars left. So I generally agree to lift the limit, but not too high and not uncon- ditionally.
Looking at what has been written in this thread so far, I see three major cases that people care about:
- Function signatures,
- Code lines and
- Code lines containing string literals[1].
Let's assume 72 chars of visible width (line length minus the inden- tation) is enough for code lines. If we say two additional indentation levels are always reasonable (a third might be too, here and there) we would be at 96 chars.
So I would compromise as follows:
o Set a hard limit around 100 chars (96 would be a nice number). o If a line doesn't contain a string literal, recommend a visible width <= 72 chars.
Nico
[1] I think we should treat the latter two separately because you can easily argue that lines with a string literal might get too long but if you apply the same rules to regular code, you'll get all the crappy stuff (too many levels of indentation, unnecessary long, less distinctive identifiers etc.).
-- coreboot mailing list: coreboot@coreboot.org https://mail.coreboot.org/mailman/listinfo/coreboot