[arch-dev-public] Let's agree on a common coding style
Hi all, while looking at our devtools, other scripts we have written together and even PKGBUILDs, you will see as much different types of indention, bracing etc. as there are authors. This always annoyed me and as I just watched Greg's talk at fosdem about committing kernel patches I'll go ahead and will start with devtools. I would suggest to use this coding style: * indent with tabs * tabs have 8 characters * don't use more than 132 columns * opening braces are top right, closing are bottom left: function foo() { echo bar } * if and for statements are like this: if true; then do something else do something else fi for i in a b c; do echo $i done * use single quotes if a string does not parseable content We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much. Greetings, Pierre -- Pierre Schmitz, https://users.archlinux.de/~pierre
On Sat, 13 Feb 2010 16:32:24 +0100 Pierre Schmitz <pierre@archlinux.de> wrote:
Hi all,
while looking at our devtools, other scripts we have written together and even PKGBUILDs, you will see as much different types of indention, bracing etc. as there are authors.
This always annoyed me and as I just watched Greg's talk at fosdem about committing kernel patches I'll go ahead and will start with devtools.
I would suggest to use this coding style:
* indent with tabs +1 * tabs have 8 characters
isn't the point of indentation with tabs just that everyone can choose the width of the representation for himself? unless you plan to use tabs for alignment, which i don't like. tabs for indentation, spaces for alignment imho.
* don't use more than 132 columns where does this number come from? * opening braces are top right, closing are bottom left: +1
function foo() { echo bar }
only for bash/C. for sh, leave out the 'function' part (it's a bashism)
* if and for statements are like this:
if true; then do something else do something else fi
for i in a b c; do echo $i done
fine by me
* use single quotes if a string does not parseable content
+1
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
source is a bashism. '.' is sh, but source is easier to grep for, so i suggest to use 'source' when the script is bash and you're pretty sure it will stay bash. '.' otherwise. ${bar} should only be used when it's needed. Dieter
Am Samstag, 13. Februar 2010 16:55:57 schrieb Dieter Plaetinck:
* tabs have 8 characters
isn't the point of indentation with tabs just that everyone can choose the width of the representation for himself? unless you plan to use tabs for alignment, which i don't like. tabs for indentation, spaces for alignment imho.
If everybody could use his own interpretation of how width tabs are shown the maximum column restriction gets pointless.
* don't use more than 132 columns
where does this number come from?
historical like the 80 columns from the vt100 and such terminals
only for bash/C. for sh, leave out the 'function' part (it's a bashism)
of course I meant without the function part; no idea why I wrote it there.
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
source is a bashism. '.' is sh, but source is easier to grep for, so i suggest to use 'source' when the script is bash and you're pretty sure it will stay bash. '.' otherwise.
good point
${bar} should only be used when it's needed.
I have setup a wiki page: http://wiki.archlinux.org/index.php/DeveloperWiki:Bash_Coding_Style And yes, this should just for bash and should appy to all our scripts including init scripts etc.. For PKGBUILD we might want to add some more specific extensions to this (I suggest a separate page for these) -- Pierre Schmitz, https://users.archlinux.de/~pierre
On Sat, 13 Feb 2010 17:33:26 +0100 Pierre Schmitz <pierre@archlinux.de> wrote:
Am Samstag, 13. Februar 2010 16:55:57 schrieb Dieter Plaetinck:
* tabs have 8 characters
isn't the point of indentation with tabs just that everyone can choose the width of the representation for himself? unless you plan to use tabs for alignment, which i don't like. tabs for indentation, spaces for alignment imho.
If everybody could use his own interpretation of how width tabs are shown the maximum column restriction gets pointless.
aha, okay i see. so 'spaces for alignment' is ok? Dieter
Am Samstag, 13. Februar 2010 17:48:40 schrieb Dieter Plaetinck:
aha, okay i see. so 'spaces for alignment' is ok?
You mean for e.g. multi line output? I'd even say only spaces are valid here. -- Pierre Schmitz, https://users.archlinux.de/~pierre
Am 13.02.2010 16:55, schrieb Dieter Plaetinck:
* indent with tabs +1 * tabs have 8 characters
isn't the point of indentation with tabs just that everyone can choose the width of the representation for himself? unless you plan to use tabs for alignment, which i don't like. tabs for indentation, spaces for alignment imho.
This affects how exactly you count the "number of columns" in the next rule. A tab is then counted as 8 characters (although it will still show as 2 in my editor).
* don't use more than 132 columns where does this number come from? * opening braces are top right, closing are bottom left: +1
function foo() { echo bar }
only for bash/C. for sh, leave out the 'function' part (it's a bashism)
* if and for statements are like this:
if true; then do something else do something else fi
for i in a b c; do echo $i done
fine by me
* use single quotes if a string does not parseable content
+1
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
Never use `...`, always $(...), always use the braces for variable names and always quote paths that contain variables.
On Sat, 2010-02-13 at 16:55 +0100, Dieter Plaetinck wrote:
source is a bashism. '.' is sh, but source is easier to grep for, so i suggest to use 'source' when the script is bash and you're pretty sure it will stay bash. '.' otherwise. ${bar} should only be used when it's needed.
source is bashism and shouldn't be used if there's a plain posix-style alternative. Also, why would you grep for "source" in a PKGBUILD if it will just return all PKGBUILDs because they contain source= lines. As for ${bar}: I would like to have that enforced for consistency.
On Sat, 13 Feb 2010 22:44:59 +0100 Thomas Bächler <thomas@archlinux.org> wrote:
Am 13.02.2010 22:13, schrieb Dieter Plaetinck:
If you use braces only when necessary, it will not be consistent - you have braces sometimes and other times you don't. I thought coding style was about consistency.
you can apply "only when needed" consistently.
This is very unintuitive. "only when needed" can even depend on the context, I am strictly against it.
what do you mean context? it only depends on whether the first character after the variablename is a valid character in a variablename or not. if it's valid, use braces. if it isn't, no need for braces. On Sat, 13 Feb 2010 22:48:54 +0100 Jan de Groot <jan@jgc.homeip.net> wrote:
On Sat, 2010-02-13 at 16:55 +0100, Dieter Plaetinck wrote:
source is a bashism. '.' is sh, but source is easier to grep for, so i suggest to use 'source' when the script is bash and you're pretty sure it will stay bash. '.' otherwise. ${bar} should only be used when it's needed.
source is bashism and shouldn't be used if there's a plain posix-style alternative. Also, why would you grep for "source" in a PKGBUILD if it will just return all PKGBUILDs because they contain source= lines.
we were not talking about only PKGBUILDS, but about all bash code.
On 14/02/10 01:32, Pierre Schmitz wrote:
Hi all,
while looking at our devtools, other scripts we have written together and even PKGBUILDs, you will see as much different types of indention, bracing etc. as there are authors.
This always annoyed me and as I just watched Greg's talk at fosdem about committing kernel patches I'll go ahead and will start with devtools.
I would suggest to use this coding style:
* indent with tabs * tabs have 8 characters * don't use more than 132 columns * opening braces are top right, closing are bottom left:
function foo() { echo bar }
* if and for statements are like this:
if true; then do something else do something else fi
for i in a b c; do echo $i done
These all agree with the makepkg coding style (apart from the "function" part which has been adjusted) so I am fine with this.
* use single quotes if a string does not parseable content
Meh, do not really make a difference.
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
Backticks suck because they are non-nestable. Always $(). Braces I do not care about. This was added to the guidelines wiki page: Add these lines at the bottom of the document to enforce our guideline: # vim: set noexpandtab tabstop=8 shiftwidth=8 textwidth=132 autoindent # kate: indent-mode normal; indent-width 8; tab-indents on; tab-width 8; word-wrap on; word-wrap-column 132 What about adding ones for emacs, geany, eclipse, ... I.e., I think adding these a waste of time as it will always be an incomplete list. How about test style: FOO=0 if [ $FOO - eq 0 ] vs if (( ! $FOO )) makepkg has switched to the later. In then end, none of our coding projects is really big (apart from pacman which has its own guidelines) so as long as indentation is consistent within a project (tabs vs spaces), I could not care less... Allan
As an aside, big patches that change coding style and touch the majority of the file are really, really annoying when using "git blame". I know you can work around it but... Allan
Am Sonntag, 14. Februar 2010 00:42:21 schrieb Allan McRae:
As an aside, big patches that change coding style and touch the majority of the file are really, really annoying when using "git blame". I know you can work around it but...
Allan
I don't see another way around this. Although I never found those blame functions of vcs of any use I agree that such commits make the history quite unreadable. That's why you define a coding guideline beforehand and you should be a little picky about it. But in this case I don't see another option; but those scripts are not that complex that this matters a lot. -- Pierre Schmitz, https://users.archlinux.de/~pierre
Am Sonntag, 14. Februar 2010 00:29:02 schrieb Allan McRae:
What about adding ones for emacs, geany, eclipse, ... I.e., I think adding these a waste of time as it will always be an incomplete list.
Yes, I wasn't sure about that either. Those lines should cover most editors though as some are compatible to the one or other. But I think I'll remove that as it's not that important and might even be too strict. We should rather go with "Use what ever you like as long as the resulting patch applies to our coding guideline".
How about test style: FOO=0 if [ $FOO - eq 0 ] vs if (( ! $FOO )) makepkg has switched to the later.
Are those tests really equivalent? Doesn't the second match also if FOO is false, an empty string or is not set at all? (but I might mix this up with other languages)
In then end, none of our coding projects is really big (apart from pacman which has its own guidelines) so as long as indentation is consistent within a project (tabs vs spaces), I could not care less...
I beg to differ. I would really like to end up with at least a recommondation for all bash scripts we have written in Arch. This not only include devtools, dbscripts and makepkg but also initscripts, wrapper scripts and in the end even PKGBUILDs. Of course, none of those files are really complex on its own; but a coding guide line is really helpful if more than one person contributes. ATM we have all kinds of different styles even within one source file. It just makes maintenance and reading the code harder. It also looks ugly ;-) One thing I have seen a lot (I might be guilty here myself) is e.g. if a packages was adopted by another maintainer one of the first commits is about changing indention etc.. -- Pierre Schmitz, https://users.archlinux.de/~pierre
On Sat, Feb 13, 2010 at 5:29 PM, Allan McRae <allan@archlinux.org> wrote:
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
Backticks suck because they are non-nestable. Always $(). Braces I do not care about.
Additionally, backticks are kinda-sorta deprecated. This means that all the people who write the standards have said "Yeah, backticks suck and the next standard will deprecate them" but standards bodies are slow as hell, so they haven't made a new standard that deprecates them yet
This was added to the guidelines wiki page: Add these lines at the bottom of the document to enforce our guideline:
# vim: set noexpandtab tabstop=8 shiftwidth=8 textwidth=132 autoindent # kate: indent-mode normal; indent-width 8; tab-indents on; tab-width 8; word-wrap on; word-wrap-column 132
What about adding ones for emacs, geany, eclipse, ... I.e., I think adding these a waste of time as it will always be an incomplete list.
I think we need at least vim and emacs, though we can trim the vim line: # vim: set noet ts=8 sw=8 tw=132 Aside: setting autoindent in a modeline is missing the point of modelines. They're intended to describe the FORMAT of the file, not how you should edit it.
How about test style: FOO=0 if [ $FOO - eq 0 ] vs if (( ! $FOO )) makepkg has switched to the later.
In then end, none of our coding projects is really big (apart from pacman which has its own guidelines) so as long as indentation is consistent within a project (tabs vs spaces), I could not care less...
This is bash specific, is it not? In projects such as initscripts we'd want to steer away from this as, in theory, we could swtich to dash some day.
On Sat, Feb 13, 2010 at 9:32 AM, Pierre Schmitz <pierre@archlinux.de> wrote:
Hi all,
while looking at our devtools, other scripts we have written together and even PKGBUILDs, you will see as much different types of indention, bracing etc. as there are authors.
This always annoyed me and as I just watched Greg's talk at fosdem about committing kernel patches I'll go ahead and will start with devtools.
I would suggest to use this coding style:
* indent with tabs * tabs have 8 characters * don't use more than 132 columns * opening braces are top right, closing are bottom left:
Ew. I prefer to write bash like python. 4 space indentation. If you want to enforce using tabs, PLEASE add modelines to the files, as my editor defaults to 4 space indentation. I agree with the braces. The width thing... yeah that's fine. 80 chars should be enough for bash, though. However, wide terminals have existed since the 70s so I see no issue with using 132 chars.
* if and for statements are like this:
Fine. I assume you're trying to point out that they SHOULDN'T be done like: if true then echo "Yay" fi Correct?
* use single quotes if a string does not parseable content
That seems a little nitpick-y to me. What's the point of calling foul if I write: echo "Hello, Pierre"
We could also talk about using `` or $(); source vs. .; $foo vs. ${bar} etc.. But that's probably too much.
Agreed. However, if we want to go with "minimum support" and get down to the ash/dash level, then it's important.
participants (6)
-
Aaron Griffin
-
Allan McRae
-
Dieter Plaetinck
-
Jan de Groot
-
Pierre Schmitz
-
Thomas Bächler