Hi,
we're hitting the 80 column limit in our code in ways which actually reduce readability for the code. Examples are various multiline messages and complicated nested code where refactoring to a separate function doesn't make sense.
Keeping the old 80 column limit is not really an option anymore. Standard terminal sizes have one of 80, 100 or 132 columns. Given the monitor resolutions many people have nowadays, I think it is safe to say that you can fit two xterms with 100 columns horizonally next to each other. 100 columns should also be sufficient for a msg_p* of roughly 80 columns of text. 132 columns provide more leeway, but IMHO that would be too wide for good readability (and my screen can't fit two xterms side-by-side anymore).
Of course some files have sections where any column limit is not acceptable (board lists etc.), but the column limit violations should be limited to the affected file sections, not whole files.
Comments? I'd like to get this decided today or tomorrow so we know where we need line breaks in Stefan Tauner's new struct flashchip patch.
Regards, Carl-Daniel
On Sun, 11 Mar 2012 16:51:08 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Hi,
we're hitting the 80 column limit in our code in ways which actually reduce readability for the code. Examples are various multiline messages and complicated nested code where refactoring to a separate function doesn't make sense.
Keeping the old 80 column limit is not really an option anymore. Standard terminal sizes have one of 80, 100 or 132 columns. Given the monitor resolutions many people have nowadays, I think it is safe to say that you can fit two xterms with 100 columns horizonally next to each other. 100 columns should also be sufficient for a msg_p* of roughly 80 columns of text. 132 columns provide more leeway, but IMHO that would be too wide for good readability (and my screen can't fit two xterms side-by-side anymore).
Of course some files have sections where any column limit is not acceptable (board lists etc.), but the column limit violations should be limited to the affected file sections, not whole files.
Comments? I'd like to get this decided today or tomorrow so we know where we need line breaks in Stefan Tauner's new struct flashchip patch.
i would prefer more complex rules than a simple "break the line at a maximum of n chars or die". increasing the hard limit would lower the frequency for anger-inducing line breaks on my side a bit, but won't solve the problem completely. i think i once proposed something like the following after you went offline, so i am repeating it now in an RFC.
x chars soft limit (e.g. 90); y chars hard limit (e.g. 120); o chars overhang (< 5 or "common sense").
before reaching x one should try to break at a convenient spot like logical operators for multi-term conditions, parameter names etc just like one usually does before hard limits. most printed text strings should have their line breaks before x at usual indention depths ((1 to 3)*8).
expressions that fit in y chars completely, but not in x should *not* be split.
same exceptions for long tables as used so far.
optional: expressions that fit in y + o chars, should not be split, just for the sake of obeying the limit i.e. if it would make the expression less readable (e.g. the case where ";" breaks the limit and i start swearing :P)
optional #2: neither limit should apply to printed text strings. they should be broken at "\n" no matter how far that is. the rationale is that it is possible to see if they are sanely formatted without runtime tests.
all of that could be replaced with a simple "we want to limit lines to ca. y chars, please think a bit and use common sense when breaking lines" rule imho. hard limits can never have enough rules/exceptions to match "common sense". of course i am ignoring personal preferences here, but i think i can cope better with "annoying" preferences of others when reviewing patches or getting feedback than with hard limits that make me write lines like "\tab\tab\tab i) {". :)
On Sun, Mar 11, 2012 at 3:00 PM, Stefan Tauner < stefan.tauner@student.tuwien.ac.at> wrote:
all of that could be replaced with a simple "we want to limit lines to ca. y chars, please think a bit and use common sense when breaking lines" rule imho. hard limits can never have enough rules/exceptions to match "common sense". of course i am ignoring personal preferences here, but i think i can cope better with "annoying" preferences of others when reviewing patches or getting feedback than with hard limits that make me write lines like "\tab\tab\tab i) {". :)
+1. Hard limits will never quite capture every rule and exception to match common sense. I dislike militant enforcement of code style, but I think 80 columns (with 8-char indentation) is a useful soft limit to aspire to for encouraging concise code. I think I'd rather see the 80 column soft-limit broken only when it makes sense to do so (arbitrary, I know), preferably only in affected areas (e.g. tables).
Breaking the soft limit, whether it's to nest deeper or add a huge print statement to Flashrom's already verbose output, should take some careful consideration but should not distract from the ultimate goal of readability and maintainability.
Am Sonntag, den 11.03.2012, 23:00 +0100 schrieb Stefan Tauner:
Keeping the old 80 column limit is not really an option anymore. Standard terminal sizes have one of 80, 100 or 132 columns. Given the monitor resolutions many people have nowadays, I think it is safe to say that you can fit two xterms with 100 columns horizonally next to each other. 100 columns should also be sufficient for a msg_p* of roughly 80 columns of text. 132 columns provide more leeway, but IMHO that would be too wide for good readability (and my screen can't fit two xterms side-by-side anymore).
Of course some files have sections where any column limit is not acceptable (board lists etc.), but the column limit violations should be limited to the affected file sections, not whole files.
Comments? I'd like to get this decided today or tomorrow so we know where we need line breaks in Stefan Tauner's new struct flashchip patch.
i would prefer more complex rules than a simple "break the line at a maximum of n chars or die". increasing the hard limit would lower the frequency for anger-inducing line breaks on my side a bit, but won't solve the problem completely.
The idea of line length limits is of course not to make anyone angry, but to make the code readable in a "typical" editing situation. Even if my window is just one or two characters too small, the last character gets chopped off at the right margin of the window. So there is a kind of hard limit for the person trying to read the code. But as windows can be resized, the limit is still kind-of soft. Defining some "window size that makes you able to see all the code" still would be a good idea to prevent a slow creep of steadily increasing line lenghts (like "Hey, it's just one char longer than this other line, so why is this line forbidden?!")
Also having strings in the source as they are printed is a good idea, so we need at least 2*tabsize + strlen("msg_perr("") + strlen("");") = 29 more chars than 80 characters (actually, for adding a "\n", even a bit more, but you seldomly hit the 80 exactly, so to have messages in a once indented block in a function body, you need 110 chars.
On the other hand, the estimate of 200 chars/screen is quite reasonable for 4:3 monitors (which are still in use by a lot of programmers), so 100 chars to put to terminals next to each other sound sensible, too.
i think i once proposed something like the following after you went offline, so i am repeating it now in an RFC.
x chars soft limit (e.g. 90); y chars hard limit (e.g. 120); o chars overhang (< 5 or "common sense").
before reaching x one should try to break at a convenient spot like logical operators for multi-term conditions, parameter names etc just like one usually does before hard limits. most printed text strings should have their line breaks before x at usual indention depths ((1 to 3)*8).
expressions that fit in y chars completely, but not in x should *not* be split.
This means I need to have the window y chars wide to be able to read the expression. So the soft limit of x does not really help. Thus I don't get what the advantage of a soft limit so much lower than the hard limit is useful for.
optional: expressions that fit in y + o chars, should not be split, just for the sake of obeying the limit i.e. if it would make the expression less readable (e.g. the case where ";" breaks the limit and i start swearing :P)
And here you are kind-of re-inventing the soft-limit/hard-limit split, but this time with a 5 char difference instead of a 30 char difference. If the stuff that is not fitting my window is just a semicolon after a function call with unused result, you can be quite sure that what is "lost" is just a semicolon. For a printing function, even if the window is just up to the closing quote, the closing parenthesis and the semicolon need not be visible to understand the line. So a small overhang indeed makes sense, if you consider it as: "If I don't see the characters in the overhang, can I still be quite confident I know what the code does?" If you can answer that question with yes, use the overhang.
optional #2: neither limit should apply to printed text strings. they should be broken at "\n" no matter how far that is. the rationale is that it is possible to see if they are sanely formatted without runtime tests.
Makes sense too, if you require that the varargs for a printf-like function again are fitting the limit. Having to scroll the window to read a detailed message is fine with me, as long as everything except string constants is completely understandable without scrolling.
here, but i think i can cope better with "annoying" preferences of others when reviewing patches or getting feedback than with hard limits that make me write lines like "\tab\tab\tab i) {". :)
Kind-of far-fetched example. Usually you should have a better line break point before that. Only exception where this might not be the case is: msg_pinfo("%d frobbing fizzles of furious foxes failed. Try teasing turkeys to\n" "testify the truth. Ignoring irratating ibises is immensely important\n", i); In this case, the only sensible line-break point really *is* before the i. As an occasional reader of that code, I might not be interested whether teasing turkeys or teasing tortoises is suggested, but I am more likely interested in knowing what value is printed. If the "i" gets pushed out of the window, I loose important information. So, yes, in this case, I am all for having "\tab\tab\tab i);" on a line. The comma may disappear past the right-border, as that comma is obviously there if the code compiles.
So my 2 cents for the discussion is: - we should have a "suggested window size" (kind of equals your hard limit. - only "obvious" characters may be past that window size - verbose hints (where flashrom tries to write novels to the screen) are unaffected of the suggested window size, but the output needs to fit in 80 chars. with following softeninig applied - If some section of code is way more legible if you get some extra chars, take them. not being able to read a code side-by-side without horizontal scrolling of the window is the lesser evil compared to having lots of obnoxious line breaks. - BUT: Do not assume if you exceeded this limit by 3 chars according to the previous rule this is generally the new suggested window size! - Plain string constants may exceed the suggested window size in the "flashrom writes a novel" case. If the message itself is not multi-line, but just on one line, break it, if you have to break the line anyway for the printf varargs. - Use whatever space you need for tables, but don't make them wider than necessary.
For the last item about making tables not wider than necessary I suggest to not use TABs inside a table, but *only* for indention of code blocks. This enables changing the tab size without everything falling apart - you just get a different indent steps. The consequence for tables is that if some column is determined to take 11 characters (including comma and space), you can just use them instead of needing to use 16 characters to get back to the TAB grid. If in a table one column entry is excessively long (e.g. a board name like "P9XGH3-Deluxe/Pro or P9XGH4-basic", this does not necessarily mean this excessive entry determines column width. Especially if it is followed by filler entries like NULL or 0 which can be squashed below nominal column width, so interesting fields do again line up.
For the "suggested window size, I think carldani's value 100 would be a good start.
Regards, Michael Karcher
Am 12.03.2012 22:23 schrieb Michael Karcher:
Am Sonntag, den 11.03.2012, 23:00 +0100 schrieb Stefan Tauner:
Keeping the old 80 column limit is not really an option anymore. Standard terminal sizes have one of 80, 100 or 132 columns. Given the monitor resolutions many people have nowadays, I think it is safe to say that you can fit two xterms with 100 columns horizonally next to each other. 100 columns should also be sufficient for a msg_p* of roughly 80 columns of text. 132 columns provide more leeway, but IMHO that would be too wide for good readability (and my screen can't fit two xterms side-by-side anymore).
Of course some files have sections where any column limit is not acceptable (board lists etc.), but the column limit violations should be limited to the affected file sections, not whole files.
i would prefer more complex rules than a simple "break the line at a maximum of n chars or die". increasing the hard limit would lower the frequency for anger-inducing line breaks on my side a bit, but won't solve the problem completely.
The idea of line length limits is of course not to make anyone angry, but to make the code readable in a "typical" editing situation. Even if my window is just one or two characters too small, the last character gets chopped off at the right margin of the window. So there is a kind of hard limit for the person trying to read the code. But as windows can be resized, the limit is still kind-of soft. Defining some "window size that makes you able to see all the code" still would be a good idea to prevent a slow creep of steadily increasing line lenghts (like "Hey, it's just one char longer than this other line, so why is this line forbidden?!")
Also having strings in the source as they are printed is a good idea, so we need at least 2*tabsize + strlen("msg_perr("") + strlen("");") = 29 more chars than 80 characters (actually, for adding a "\n", even a bit more, but you seldomly hit the 80 exactly, so to have messages in a once indented block in a function body, you need 110 chars.
My screen can fit two xterms with exactly 112 columns each next to each other, so I like this suggestion.
On the other hand, the estimate of 200 chars/screen is quite reasonable for 4:3 monitors (which are still in use by a lot of programmers), so 100 chars to put to terminals next to each other sound sensible, too.
100 columns is of course nice as well.
i think i once proposed something like the following after you went offline, so i am repeating it now in an RFC.
x chars soft limit (e.g. 90); y chars hard limit (e.g. 120); o chars overhang (< 5 or "common sense").
before reaching x one should try to break at a convenient spot like logical operators for multi-term conditions, parameter names etc just like one usually does before hard limits. most printed text strings should have their line breaks before x at usual indention depths ((1 to 3)*8). expressions that fit in y chars completely, but not in x should *not* be split.
This means I need to have the window y chars wide to be able to read the expression. So the soft limit of x does not really help. Thus I don't get what the advantage of a soft limit so much lower than the hard limit is useful for.
optional: expressions that fit in y + o chars, should not be split, just for the sake of obeying the limit i.e. if it would make the expression less readable (e.g. the case where ";" breaks the limit and i start swearing :P)
And here you are kind-of re-inventing the soft-limit/hard-limit split, but this time with a 5 char difference instead of a 30 char difference. If the stuff that is not fitting my window is just a semicolon after a function call with unused result, you can be quite sure that what is "lost" is just a semicolon. For a printing function, even if the window is just up to the closing quote, the closing parenthesis and the semicolon need not be visible to understand the line. So a small overhang indeed makes sense, if you consider it as: "If I don't see the characters in the overhang, can I still be quite confident I know what the code does?" If you can answer that question with yes, use the overhang.
And I'm totally against having 4 kinds of limits: extremely soft (Stefan's soft limit), soft (Stefan's hard limit), hard (Stefan's hard+overhang) and none (for tables). The no-limits exception for tables is good. No objection there. Everybody is free to introduce linebreaks before any limit if it improves readability. Admittedly those linebreaks will disappear if someone runs indent on the code and commits it as "trivial patch" - we had quite a few such commits in the past (and I was not really happy about that). Other than for tables, I really want a hard limit with no exceptions because I'm pretty sure there will be quite some disagreement about what constitutes being "confident about what the code does". For me, "\n" or not is a question which has an impact on output, especially when I'm hunting for empty lines which got printed somewhere. Even more so if we're talking about parts of a printed string which contain format strings. If I have to resize the terminal anyway to see the complete code, I might as well make the most of it and not waste lines by introducing linebreaks elsewhere.
optional #2: neither limit should apply to printed text strings. they should be broken at "\n" no matter how far that is. the rationale is that it is possible to see if they are sanely formatted without runtime tests.
Makes sense too, if you require that the varargs for a printf-like function again are fitting the limit. Having to scroll the window to read a detailed message is fine with me, as long as everything except string constants is completely understandable without scrolling.
here, but i think i can cope better with "annoying" preferences of others when reviewing patches or getting feedback than with hard limits that make me write lines like "\tab\tab\tab i) {". :)
Kind-of far-fetched example. Usually you should have a better line break point before that. Only exception where this might not be the case is: msg_pinfo("%d frobbing fizzles of furious foxes failed. Try teasing turkeys to\n" "testify the truth. Ignoring irratating ibises is immensely important\n", i); In this case, the only sensible line-break point really *is* before the i. As an occasional reader of that code, I might not be interested whether teasing turkeys or teasing tortoises is suggested, but I am more likely interested in knowing what value is printed. If the "i" gets pushed out of the window, I loose important information. So, yes, in this case, I am all for having "\tab\tab\tab i);" on a line. The comma may disappear past the right-border, as that comma is obviously there if the code compiles.
So my 2 cents for the discussion is:
- we should have a "suggested window size" (kind of equals your hard
limit.
- only "obvious" characters may be past that window size
- verbose hints (where flashrom tries to write novels to the screen) are unaffected of the suggested window size, but the output needs to fit in 80 chars.
with following softeninig applied
- If some section of code is way more legible if you get some extra chars, take them. not being able to read a code side-by-side without horizontal scrolling of the window is the lesser evil compared to having lots of obnoxious line breaks.
- BUT: Do not assume if you exceeded this limit by 3 chars according to the previous rule this is generally the new suggested window size!
- Plain string constants may exceed the suggested window size in the "flashrom writes a novel" case. If the message itself is not multi-line, but just on one line, break it, if you have to break the line anyway for the printf varargs.
And what line length would be the real hard (no exceptions except for tables) limit? The softening of limits just makes it harder to find out what the real limit is, and I don't think there is any indentation tool (even the stuff which is not as crappy as GNU indent) which has enough atificial intelligence to find out what's considered to be "obvious" content allowed after the limit.
- Use whatever space you need for tables, but don't make them wider than necessary.
For the last item about making tables not wider than necessary I suggest to not use TABs inside a table, but *only* for indention of code blocks. This enables changing the tab size without everything falling apart - you just get a different indent steps. The consequence for tables is that if some column is determined to take 11 characters (including comma and space), you can just use them instead of needing to use 16 characters to get back to the TAB grid. If in a table one column entry is excessively long (e.g. a board name like "P9XGH3-Deluxe/Pro or P9XGH4-basic", this does not necessarily mean this excessive entry determines column width. Especially if it is followed by filler entries like NULL or 0 which can be squashed below nominal column width, so interesting fields do again line up.
Full agreement for squeezing tables in a way which does not make a table the vitcim of its longest member.
For the "suggested window size, I think carldani's value 100 would be a good start.
Regards, Carl-Daniel
Unless someone protests, I'd say 112 columns are the new absolute hard limit, and there is no exception for anything except tables. Table compression (i.e. having a few rows with unaligned elements following a super-long element) is good as long as the majority of elements in each column are aligned.
Those limits only affect new code for the next 2 months starting today, and if we're still happy with the limit after 2 months, the rest of the code will be changed gradually where it helps readability. Should we encounter code within the next 2 months where 112 columns are way worse than 120, the limit will be increased to 120 with no additional evaluation time period.
Regards, Carl-Daniel
On Sun, May 06, 2012 at 02:12:32PM +0200, Carl-Daniel Hailfinger wrote:
Unless someone protests, I'd say 112 columns are the new absolute hard limit, and there is no exception for anything except tables. Table compression (i.e. having a few rows with unaligned elements following a super-long element) is good as long as the majority of elements in each column are aligned.
Those limits only affect new code for the next 2 months starting today, and if we're still happy with the limit after 2 months, the rest of the code will be changed gradually where it helps readability. Should we encounter code within the next 2 months where 112 columns are way worse than 120, the limit will be increased to 120 with no additional evaluation time period.
I for one would very much prefer the 79/80 character (soft) limit per default.
No objections about breaking the rule for exception cases where this makes sense (large tables, long lines which cannot be split nicely for whatever reasons), but as a general guideline sticking to 79 char/line is a very good thing IMHO.
Uwe.
This topic came up in a patch submission, and it occurred to me that we never actually wrote down canonical limits in the wiki. So I went ahead and added the proposed limits here: https://www.flashrom.org/Development_Guidelines#Coding_style
It seems that we never really decided on 112-characters vs. 120-characters, so I wrote down 112-characters for now. At least most were satisfied with an 80-column "soft" limit and an exception to the limits for tables...
On Sun, May 6, 2012 at 7:10 AM Uwe Hermann uwe@hermann-uwe.de wrote:
On Sun, May 06, 2012 at 02:12:32PM +0200, Carl-Daniel Hailfinger wrote:
Unless someone protests, I'd say 112 columns are the new absolute hard limit, and there is no exception for anything except tables. Table compression (i.e. having a few rows with unaligned elements following a super-long element) is good as long as the majority of elements in each column are aligned.
Those limits only affect new code for the next 2 months starting today, and if we're still happy with the limit after 2 months, the rest of the code will be changed gradually where it helps readability. Should we encounter code within the next 2 months where 112 columns are way worse than 120, the limit will be increased to 120 with no additional evaluation time period.
I for one would very much prefer the 79/80 character (soft) limit per default.
No objections about breaking the rule for exception cases where this makes sense (large tables, long lines which cannot be split nicely for whatever reasons), but as a general guideline sticking to 79 char/line is a very good thing IMHO.
Uwe.
http://hermann-uwe.de | http://sigrok.org http://randomprojects.org | http://unmaintained-free-software.org
flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
Op 11-3-2012 16:51, Carl-Daniel Hailfinger schreef:
we're hitting the 80 column limit in our code in ways which actually reduce readability for the code. Examples are various multiline messages and complicated nested code where refactoring to a separate function doesn't make sense.
Coming from DOS as as experimentation platform, I guess there's no sensible way to take in a per-platform (or 'dos' versus 'non-dos') approach to output messages. Too much if/else, though Stefan's technical approach/algorythm sounds viable in some kind of odd way.
As a compromise I'd suggest keeping the default (non-verbose) messages within 80char limit, and everything else (verbose messages, detailed messages) beyond 80 if so desired. Already used to ugly output anyway since ReactOS started using Cmake with its full path references. (for example http://www.reactos.org/bugzilla/attachment.cgi?id=7390 )
Regards, Carl-Daniel
Is this list accepting Coreboot GSOC idea suggestions? Not subscribed at the relevant place, don't like to sign up to too many places:
1) Live compile distro (small: puppy/tinycore/partedmagic + GCC?) 2) Somehow make use of raspberry-pi with regard to coreboot 3) DRAM-less system (Coreboot/CAR + SeaBIOS + DOS on AMD APU system?).
Bernd
Am Dienstag, den 13.03.2012, 22:48 +0100 schrieb Bernd Blaauw:
Op 11-3-2012 16:51, Carl-Daniel Hailfinger schreef:
we're hitting the 80 column limit in our code in ways which actually reduce readability for the code. Examples are various multiline messages and complicated nested code where refactoring to a separate function doesn't make sense.
Coming from DOS as as experimentation platform, I guess there's no sensible way to take in a per-platform (or 'dos' versus 'non-dos') approach to output messages. Too much if/else, though Stefan's technical approach/algorythm sounds viable in some kind of odd way.
It seems you maybe missed the point of our discussion. We were not discussing the number of columns of the messages we print on the screen, but the number of columns in the source code. Output messages are still formatted to fit into 80 characters (which is not only the default width of DOS text mode, the Windows DOS box or the linux console, but also the default width of most Linux "terminal emulators" (which are in Linux graphical user interface what DOS/console boxes are in Windows).
Regards, Michael Karcher
-
Bernd Blaauw -
Carl-Daniel Hailfinger -
David Hendricks -
David Hendricks -
Michael Karcher -
Stefan Tauner -
Uwe Hermann