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:
1. Function signatures, 2. Code lines and 3. 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.).
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
Dear Nico, dear coreboot folks,
Am Freitag, den 25.05.2018, 18:40 +0200 schrieb Nico Huber:
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.
I like Nico’s proposal.
Kind regards,
Paul
As mentioned in my earlier message in this thread, I don't have an iron in this fire. I'm just trying to be helpful.
On 25/05/2018, Nico Huber nico.h@gmx.de wrote:
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.
This makes good sense except that it would seem to require the editing environment to be specially configured to recognise lines with string literals so as to be able to auto-wrap them, or to warn about them, differently to other lines. Depending upon the text editor involved, that might be non-trivial; I don't know.
On a more general note: I expect most people on this list are already aware that there have been various efforts over the years to create decent coding standards or style guides for various languages, including conventions about line length, and to translate some of them into text editor configuration files or linting tools for automated feedback. For those who weren't aware, see e.g.:
https://en.wikipedia.org/wiki/Lint_(software)
https://en.wikipedia.org/wiki/Gnits_standards
https://www.kernel.org/doc/html/v4.10/process/coding-style.html
https://github.com/google/styleguide
It seems to me that sticking to a style guide that is well-supported by text editors and linting tools would be a wise thing to do, to minimise manual labour, human error, and development environment configuration effort, even if it does mean occasionally needing to manually break string literals at 80chars or suchlike.
(I'll duck out of the conversation now.)
Am Fr., 25. Mai 2018 um 18:40 Uhr schrieb Nico Huber nico.h@gmx.de:
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.
As Sam noted, it's hard to find tooling for that. But even with 96 characters clang-format's output looks much better than with 80.
One thing your transfer of the 72 char rule from continuous text to code doesn't take into account is that in code, individual lines often stand on their own. Consider the following block:
/* this function will fill the corresponding manufacturer */ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17 *t) { switch (mod_id) { case 0x2c80: t->manufacturer = smbios_add_string(t->eos, "Crucial"); break;
You'd never set a chapter in a book like this except maybe as an art project (that I'd decline to read). It's more readable after collapsing both the header and the add_string call to a single line each (two "carriage returns" in the middle of a statement less), despite passing the 72/80 character limit - except if you're on an old-fashioned terminal where part of the line is missing because it's too long now.
If people write chapters full of documentation in comments, by all means, let's limit that to a 72 character width (plus indentation for "/* "). Code formatters generally leave comments alone if they can help it, so that requires a different tool if we want to enforce it.
Patrick
-
Nico Huber -
Patrick Georgi -
Paul Menzel -
ron minnich -
Sam Kuper