On Mon, 17 Nov 2014 13:24:21 -0800
Evan Gates <evan.gates_AT_gmail.com> wrote:
Hey Evan,
> sbase has no unified style. Many things change from file to file. I am
> more annoyed by this than I should be. I propose a style document for
> sbase. I have included a starting point in order to promote
> discussion. You will disagree with some of these points. Speak up and
> explain your alternative and reasons. In the end it's up to the
> maintainers to decide what the rules are, if there are any. I just
> want to get the ball rolling.
well, the ball is already rolling. Thanks for the heads-up though again.
I must honestly say that you gotta work on your coding style a bit better.
I spent all evening yesterday and some hours this afternoon to untangle
your expr.c. It wasn't too bad, but for instance, you had unnecessary
global variables, unsafe pointer use in some places and raw errors which
could break things.
But let's look at sbase in general here. Let me give you my 2 cents about
the points presented:
> whitespace
> ==========
> tabs for indentation
> spaces for alignment
> (...)
suckless default. Write patch where you see this is broken.
> blocks
> ======
> all variable declarations at top of block
Yeah!
> { on same line preceded by single space (except functions)
> } on own line except for continuation of statement (e.g. if else, do while)
> (...)
Maybe too strict here. Depends on taste.
> branches
> (...)
> inner statement block
> (...)
suckless default. I wonder why this is kinda broken in sbase.
> keywords
> ========
> space after if/for/while/switch (it's not a function call)
> no space after (
> always use () with sizeof and no space (sizeof is like a function call)
Yep
> variables
> =========
> declaration of pointer, * is adjacent to variable not type (as it's
> part of the variable not the type, avoids int* i, j; etc.)
This is a very smart point. I see this very often, even though not
in sbase. a grouped declaration "int* a, b, c;" could falsly
indicate, that b and c are pointers, too, which is not the case.
> headers
> =======
> system headers (#include <...>) in alphabetical order
> empty line
> local headers (#include "...") in alphabetical order
> if there is any reason to change order, comment to explain
If headers depend on order, the headers are broken. Dismissed.
> variadic macros
> ===============
> yay or nay?
Why not? :P
> functions
> =========
> declarations at top of file
> declarations, definitions, in alphabetical order (except main, at end of file)
No, I prefer declarations in order. This keeps everything in a logical context.
> static if not used outside current file (what's the right term?
> compilation unit?)
scope.
> C version
> =========
> use C99 (why not C11? I really like anonymous unions/structs)
suckless default is a mix of C90 and C99. C11 is a mess.
> do not use for loop initial declarations (why?)
Well it's part of "declarations at top of file". Since I stopped using it
I found out how nicely you can recycle index-integers for multiple loops. :)
> line length
> ===========
> we aren't limited to 80 columns in our terminals anymore, no need to
> pretend we are
> if using more columns makes code more readable, do so
> it also means we can fit more code on a single screen, less scrolling
> (some hard limit so we don't have 300 character lines?)
Should be debatable. It's like with LaTeX, where you think at first why it
puts your text into such thin columns by default, but as soon as you hold
the book in your hand you realize why.
The natural reading-buffer in your brain is only so big. I'd really like to
test out how strictly following the 80 columns rule might work out.
What you need is a good rule-set on how to format multiline if-blocks and other
things.
> tests (boolean)
> ===============
> do not test against NULL explicitly (e.g. if (!p) instead of if (p == NULL))
> do not test against 0 explicitly (e.g. if (!strcmp(p, q)) instead of
> if (strcmp(p, q) == 0)) (discuss)
Yes, yes, yes. See the patches I sent in a few days ago.
> do not use bool type (helps prevent fallacies such as if (iscorrect() == TRUE))
See the patches I sent in a few days ago.
> do use compound assignment and test (e.g. if (!(p =
> malloc(sizeof(Type)))) ...; )
> unless you can assign at declaration (e.g. Type *p =
> malloc(sizeof(Type)); if (!p) ...; )
> (although that's more rare without mixed code and declarations, discuss)
See the patches I sent in a few days ago.
> switch
> ======
> indent cases to same level as switch (i.e. do not indent the body of
> the switch statement)
> comment fall through cases
Obviously. The standard format is:
switch (a) {
case 'a':
123;
default:
123;
}
>
> early returns/exits
> ===================
> if needed use label + goto for cleanup instead of multiple nested
> levels, however this can normally be avoided (discuss)
> return/exit/fail early instead of using multiple levels of tests
> do not clean up on fatal errors (just enprintf())
> do not use an else/break/etc. after returning/failing
I have no problem with goto. It often simplifies things dramatically
and there's no reason to regulate it or punish its use.
> exceptions
> ==========
> exceptions to these rules can be made in order to make the code more
> readable. e.g.:
> (I think the exceptions will be the most contentious area. discuss)
Well, sometimes it just helps turning your brain on. Defining rules is
okay, as long as you don't overdo it.
A coding style is a good thing and we're getting there.
Are you working on patches to fix the coding style in sbase? I generally
agree with you EXCEPT the ordered function prototypes in alphabetical order,
which makes no sense.
Cheers
FRIGN
--
FRIGN <dev_AT_frign.de>
Received on Mon Nov 17 2014 - 22:47:31 CET