We currently have a 80-column limit in almost all files (except database like stuff in flashchips.c etc.). In the last few months, we have added more debug messages and more verbose and thus better understandable messages. Especially at a deep nesting level, using printf_debug can mean you have less than 20 chars per line left for messages. This actively reduces code readability.
I would like to move the column limit from 80 to some other agreed-upon value, but I don't know which one. - 100 - 120 - 132
What do YOU think?
Regards, Carl-Daniel
On Tue, Dec 15, 2009 at 05:31:27AM +0100, Carl-Daniel Hailfinger wrote:
We currently have a 80-column limit in almost all files (except database like stuff in flashchips.c etc.). In the last few months, we have added more debug messages and more verbose and thus better understandable messages. Especially at a deep nesting level, using printf_debug can mean you have less than 20 chars per line left for messages. This actively reduces code readability.
I would like to move the column limit from 80 to some other agreed-upon value, but I don't know which one.
- 100
- 120
- 132
What do YOU think?
Regards, Carl-Daniel
What about 4 space indents?
Luc Verhaegen.
On 12/14/2009 8:31 PM, Carl-Daniel Hailfinger wrote:
We currently have a 80-column limit in almost all files (except database like stuff in flashchips.c etc.). In the last few months, we have added more debug messages and more verbose and thus better understandable messages. Especially at a deep nesting level, using printf_debug can mean you have less than 20 chars per line left for messages. This actively reduces code readability.
I would like to move the column limit from 80 to some other agreed-upon value, but I don't know which one.
- 100
- 120
- 132
What do YOU think?
Regards, Carl-Daniel
The reason for 80-column limit is due to historical reasons, IMO, but we could go with 4-space tabs and 100-column limit. I found with that setting almost everything can be fit on a line if need be. Anything bigger, I think there _is_ a better way to code a problem.
On 15.12.2009 21:53, Sean Nelson wrote:
On 12/14/2009 8:31 PM, Carl-Daniel Hailfinger wrote:
I would like to move the column limit from 80 to some other agreed-upon value, but I don't know which one.
- 100
- 120
- 132
The reason for 80-column limit is due to historical reasons, IMO, but we could go with 4-space tabs and 100-column limit. I found with that setting almost everything can be fit on a line if need be. Anything bigger, I think there _is_ a better way to code a problem.
I'm against replacing tabs with spaces. It increases typing effort a lot and that is painful especially if you reindent a big hunk of code (and the indent tool is unusable without manual fixups).
Regards, Carl-Daniel
That's why these arguments always get so messy.
I say "stick with lindent". Error messages getting too long? One solution that I've seen:
char *Ewhatever = "whatever"
Then in the print you do: fprintf(stderr, "Ewhatever");
I think once you open this discussion up you open it up completely, as we can see.
I also think going to four space indent was one of the huge Python mistakes, but that's just me :-)
If you want to open it up just a little to solve this current problem, one possibilty is this: "lindent style with arbitrary length lines"
80 columns --> a standard set in 1890. do we need that?
Is it also possible that the real problem is that some things ought to be functions that are not?
ron
On 12/15/2009 2:24 PM, Carl-Daniel Hailfinger wrote:
On 15.12.2009 21:53, Sean Nelson wrote:
On 12/14/2009 8:31 PM, Carl-Daniel Hailfinger wrote:
I would like to move the column limit from 80 to some other agreed-upon value, but I don't know which one.
- 100
- 120
- 132
The reason for 80-column limit is due to historical reasons, IMO, but we could go with 4-space tabs and 100-column limit. I found with that setting almost everything can be fit on a line if need be. Anything bigger, I think there _is_ a better way to code a problem.
I'm against replacing tabs with spaces. It increases typing effort a lot and that is painful especially if you reindent a big hunk of code (and the indent tool is unusable without manual fixups).
Regards, Carl-Daniel
I don't know if its possible, I think vim and TextMate (the two editors I use all the time) allows setting the style of tabs. And I've seen vim comments for files that set the 4-column tabs. I don't really care what we use.
I only knew that dot matrix printers and old computer terminals used the 80 columns-width. I didn't know that its a standard from the 1890s. Thanks Ron.
On Tue, Dec 15, 2009 at 3:58 PM, Sean Nelson audiohacked@gmail.com wrote:
I only knew that dot matrix printers and old computer terminals used the 80 columns-width. I didn't know that its a standard from the 1890s. Thanks Ron.
Apologies, I was wrong, it only goes back to 1928 :-)
But we sure don't want to limit ourselves to an 80-year-old standard :-)
ron
On Tue, Dec 15, 2009 at 10:33:48PM -0800, ron minnich wrote:
On Tue, Dec 15, 2009 at 3:58 PM, Sean Nelson audiohacked@gmail.com wrote:
I only knew that dot matrix printers and old computer terminals used the 80 columns-width. I didn't know that its a standard from the 1890s. Thanks Ron.
Apologies, I was wrong, it only goes back to 1928 :-)
But we sure don't want to limit ourselves to an 80-year-old standard :-)
ron
Well, i personally try to stick close to the 80char standard, but i am not religiously stuck to it, as was witnessed in the board enable table discussion.
I prefer sticking to 80 chars where it does not hurt, and it should be trivially possible to stick to 80 chars in most cases. Not sure what exact line causes this now, but i am sure that it can be solved: strings can be broken up trivially, and we should all by now be used to broken up strings.
Luc Verhaegen.
Hi,
On Wed, Dec 16, 2009 at 09:23:44AM +0100, Luc Verhaegen wrote:
I only knew that dot matrix printers and old computer terminals used the 80 columns-width. I didn't know that its a standard from the 1890s. Thanks Ron.
Apologies, I was wrong, it only goes back to 1928 :-)
But we sure don't want to limit ourselves to an 80-year-old standard :-)
ron
Well, i personally try to stick close to the 80char standard, but i am
Ditto.
not religiously stuck to it, as was witnessed in the board enable table discussion.
I prefer sticking to 80 chars where it does not hurt, and it should be trivially possible to stick to 80 chars in most cases. Not sure what
I fully agree.
exact line causes this now, but i am sure that it can be solved: strings can be broken up trivially, and we should all by now be used to broken up strings.
Yep.
There is nothing historic about this limitation. My console is still 80x25 (and yes, I do use it for coding from time to time), and all my xterms are also 80x25 when I work in X11 (using vim in an xterm), so I'm strongly in favor of trying to keep the 80 chars limit where possible (and also keeping indentation to one tab, as per coding style).
Also, as the coding style document we follow (the Linux one) states very nicely, if you need more that 80 chars per line, that's usually a good indicator that the code is nested too much and should be refactored. I fully agree with that statement.
Of course there are exceptions (overly long printf's that cannot be wrapped nicely, or the board-enable table), that's fine. We can live with a few exceptions that make sense. But as a general rule I'm strongly in favor of sticking to 80 chars.
I also wouldn't object a patch which makes our prints shorter, e.g. changing
printf_debug("Foo");
or
printf(MSG_DEBUG, "Foo");
to
printk(DBG, "Foo")
or
// d for DEBUG, i for INFO, e for ERROR printe("Foo"); printi("Foo"); printd("Foo");
or something like that (just an example, maybe someone can come up with a good name).
Uwe.
On 17.12.2009 11:10, Uwe Hermann wrote:
On Wed, Dec 16, 2009 at 09:23:44AM +0100, Luc Verhaegen wrote:
not religiously stuck to it, as was witnessed in the board enable table discussion.
I prefer sticking to 80 chars where it does not hurt, and it should be trivially possible to stick to 80 chars in most cases. Not sure what
I fully agree.
Yes, most cases are no problem. I am concerned with those where we have to break up strings excessively.
exact line causes this now, but i am sure that it can be solved: strings can be broken up trivially, and we should all by now be used to broken up strings.
Yep.
Also, as the coding style document we follow (the Linux one) states very nicely, if you need more that 80 chars per line, that's usually a good indicator that the code is nested too much and should be refactored. I fully agree with that statement.
Example from an upcoming patch.
@@ -743,19 +805,36 @@ int ret = 0; int oppos, preoppos; for (; (cmds->writecnt || cmds->readcnt) && !ret; cmds++) { - /* Is the next command valid or a terminator? */ if ((cmds + 1)->writecnt || (cmds + 1)->readcnt) { + /* Next command is valid. */ preoppos = find_preop(curopcodes, cmds->writearr[0]); oppos = find_opcode(curopcodes, (cmds + 1)->writearr[0]); - /* Is the opcode of the current command listed in the - * ICH struct OPCODES as associated preopcode for the - * opcode of the next command? + if ((oppos != -1) && (preoppos != -1)) { + /* Current command is listed as preopcode in + * ICH struct OPCODES and next command is listed + * as opcode in that struct. + */ + if ((curopcodes->opcode[oppos].atomic - 1) == + preoppos) { + /* They are paired up. */ + continue; + } else { + /* FIXME: We should simply adjust the + * pairing automatically. + */ + printf_debug("%s: preop 0x%02x and op " + "0x%02x are not paired up," + " executing as separate " + "ops\n", __func__, + cmds->writearr[0], + (cmds + 1)->writearr[0]); + } + } + /* FIXME: Check if this is a preop and the next command + * is an unknown op. In that case, reprogram opcode menu + * if possible. */ - if ((oppos != -1) && (preoppos != -1) && - ((curopcodes->opcode[oppos].atomic - 1) == preoppos)) - continue; - } - + } ret = ich_spi_send_command(cmds->writecnt, cmds->readcnt, cmds->writearr, cmds->readarr); }
Splitting the function above into multiple functions really doesn't make any sense, and the innermost printf_debug is really not funny anymore.
The patch hunk above was my motivator for the coding style RFC. Suggestions how to solve it are welcome.
Of course there are exceptions (overly long printf's that cannot be wrapped nicely, or the board-enable table), that's fine. We can live with a few exceptions that make sense. But as a general rule I'm strongly in favor of sticking to 80 chars.
I also wouldn't object a patch which makes our prints shorter, e.g. changing
printf_debug("Foo");
or
printf(MSG_DEBUG, "Foo");
to
printk(DBG, "Foo")
printk(DBG, "Foo") saves 1 character compared to printf_debug("Foo"), so it isn't the best choice.
or
// d for DEBUG, i for INFO, e for ERROR printe("Foo"); printi("Foo"); printd("Foo");
or something like that (just an example, maybe someone can come up with a good name).
Using msg_dbg saves 5 chars over printf_debug and it is only 1 char longer than printe/printi/printd.
Independent of the coding style issue I think using msg_dbg for debug msg_err for error msg_inf for info might be a viable solution for length reduction of prints.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Luc Verhaegen -
ron minnich -
Sean Nelson -
Uwe Hermann