mirror of
https://github.com/git/git
synced 2024-08-27 11:39:22 +00:00
Merge branch 'jc/coding-guidelines'
* jc/coding-guidelines: CodingGuidelines: avoid "test <cond> -a/-o <cond>" CodingGuidelines: on splitting a long line CodingGuidelines: on comparison CodingGuidelines: do not call the conditional statement "if()" CodingGuidelines: give an example for shell function preamble CodingGuidelines: give an example for control statements CodingGuidelines: give an example for redirection CodingGuidelines: give an example for case/esac statement CodingGuidelines: once it is in, it is not worth the code churn
This commit is contained in:
commit
aa4bffa235
|
@ -18,6 +18,14 @@ code. For Git in general, three rough rules are:
|
|||
judgement call, the decision based more on real world
|
||||
constraints people face than what the paper standard says.
|
||||
|
||||
- Fixing style violations while working on a real change as a
|
||||
preparatory clean-up step is good, but otherwise avoid useless code
|
||||
churn for the sake of conforming to the style.
|
||||
|
||||
"Once it _is_ in the tree, it's not really worth the patch noise to
|
||||
go and fix it up."
|
||||
Cf. http://article.gmane.org/gmane.linux.kernel/943020
|
||||
|
||||
Make your code readable and sensible, and don't try to be clever.
|
||||
|
||||
As for more concrete guidelines, just imitate the existing code
|
||||
|
@ -34,7 +42,17 @@ For shell scripts specifically (not exhaustive):
|
|||
|
||||
- We use tabs for indentation.
|
||||
|
||||
- Case arms are indented at the same depth as case and esac lines.
|
||||
- Case arms are indented at the same depth as case and esac lines,
|
||||
like this:
|
||||
|
||||
case "$variable" in
|
||||
pattern1)
|
||||
do this
|
||||
;;
|
||||
pattern2)
|
||||
do that
|
||||
;;
|
||||
esac
|
||||
|
||||
- Redirection operators should be written with space before, but no
|
||||
space after them. In other words, write 'echo test >"$file"'
|
||||
|
@ -43,6 +61,14 @@ For shell scripts specifically (not exhaustive):
|
|||
redirection target in a variable (as shown above), our code does so
|
||||
because some versions of bash issue a warning without the quotes.
|
||||
|
||||
(incorrect)
|
||||
cat hello > world < universe
|
||||
echo hello >$world
|
||||
|
||||
(correct)
|
||||
cat hello >world <universe
|
||||
echo hello >"$world"
|
||||
|
||||
- We prefer $( ... ) for command substitution; unlike ``, it
|
||||
properly nests. It should have been the way Bourne spelled
|
||||
it from day one, but unfortunately isn't.
|
||||
|
@ -81,14 +107,33 @@ For shell scripts specifically (not exhaustive):
|
|||
"then" should be on the next line for if statements, and "do"
|
||||
should be on the next line for "while" and "for".
|
||||
|
||||
(incorrect)
|
||||
if test -f hello; then
|
||||
do this
|
||||
fi
|
||||
|
||||
(correct)
|
||||
if test -f hello
|
||||
then
|
||||
do this
|
||||
fi
|
||||
|
||||
- We prefer "test" over "[ ... ]".
|
||||
|
||||
- We do not write the noiseword "function" in front of shell
|
||||
functions.
|
||||
|
||||
- We prefer a space between the function name and the parentheses. The
|
||||
opening "{" should also be on the same line.
|
||||
E.g.: my_function () {
|
||||
- We prefer a space between the function name and the parentheses,
|
||||
and no space inside the parentheses. The opening "{" should also
|
||||
be on the same line.
|
||||
|
||||
(incorrect)
|
||||
my_function(){
|
||||
...
|
||||
|
||||
(correct)
|
||||
my_function () {
|
||||
...
|
||||
|
||||
- As to use of grep, stick to a subset of BRE (namely, no \{m,n\},
|
||||
[::], [==], or [..]) for portability.
|
||||
|
@ -106,6 +151,19 @@ For shell scripts specifically (not exhaustive):
|
|||
interface translatable. See "Marking strings for translation" in
|
||||
po/README.
|
||||
|
||||
- We do not write our "test" command with "-a" and "-o" and use "&&"
|
||||
or "||" to concatenate multiple "test" commands instead, because
|
||||
the use of "-a/-o" is often error-prone. E.g.
|
||||
|
||||
test -n "$x" -a "$a" = "$b"
|
||||
|
||||
is buggy and breaks when $x is "=", but
|
||||
|
||||
test -n "$x" && test "$a" = "$b"
|
||||
|
||||
does not have such a problem.
|
||||
|
||||
|
||||
For C programs:
|
||||
|
||||
- We use tabs to indent, and interpret tabs as taking up to
|
||||
|
@ -149,7 +207,7 @@ For C programs:
|
|||
of "else if" statements, it can make sense to add braces to
|
||||
single line blocks.
|
||||
|
||||
- We try to avoid assignments inside if().
|
||||
- We try to avoid assignments in the condition of an "if" statement.
|
||||
|
||||
- Try to make your code understandable. You may put comments
|
||||
in, but comments invariably tend to stale out when the code
|
||||
|
@ -177,6 +235,88 @@ For C programs:
|
|||
- Double negation is often harder to understand than no negation
|
||||
at all.
|
||||
|
||||
- There are two schools of thought when it comes to comparison,
|
||||
especially inside a loop. Some people prefer to have the less stable
|
||||
value on the left hand side and the more stable value on the right hand
|
||||
side, e.g. if you have a loop that counts variable i down to the
|
||||
lower bound,
|
||||
|
||||
while (i > lower_bound) {
|
||||
do something;
|
||||
i--;
|
||||
}
|
||||
|
||||
Other people prefer to have the textual order of values match the
|
||||
actual order of values in their comparison, so that they can
|
||||
mentally draw a number line from left to right and place these
|
||||
values in order, i.e.
|
||||
|
||||
while (lower_bound < i) {
|
||||
do something;
|
||||
i--;
|
||||
}
|
||||
|
||||
Both are valid, and we use both. However, the more "stable" the
|
||||
stable side becomes, the more we tend to prefer the former
|
||||
(comparison with a constant, "i > 0", is an extreme example).
|
||||
Just do not mix styles in the same part of the code and mimic
|
||||
existing styles in the neighbourhood.
|
||||
|
||||
- There are two schools of thought when it comes to splitting a long
|
||||
logical line into multiple lines. Some people push the second and
|
||||
subsequent lines far enough to the right with tabs and align them:
|
||||
|
||||
if (the_beginning_of_a_very_long_expression_that_has_to ||
|
||||
span_more_than_a_single_line_of ||
|
||||
the_source_text) {
|
||||
...
|
||||
|
||||
while other people prefer to align the second and the subsequent
|
||||
lines with the column immediately inside the opening parenthesis,
|
||||
with tabs and spaces, following our "tabstop is always a multiple
|
||||
of 8" convention:
|
||||
|
||||
if (the_beginning_of_a_very_long_expression_that_has_to ||
|
||||
span_more_than_a_single_line_of ||
|
||||
the_source_text) {
|
||||
...
|
||||
|
||||
Both are valid, and we use both. Again, just do not mix styles in
|
||||
the same part of the code and mimic existing styles in the
|
||||
neighbourhood.
|
||||
|
||||
- When splitting a long logical line, some people change line before
|
||||
a binary operator, so that the result looks like a parse tree when
|
||||
you turn your head 90-degrees counterclockwise:
|
||||
|
||||
if (the_beginning_of_a_very_long_expression_that_has_to
|
||||
|| span_more_than_a_single_line_of_the_source_text) {
|
||||
|
||||
while other people prefer to leave the operator at the end of the
|
||||
line:
|
||||
|
||||
if (the_beginning_of_a_very_long_expression_that_has_to ||
|
||||
span_more_than_a_single_line_of_the_source_text) {
|
||||
|
||||
Both are valid, but we tend to use the latter more, unless the
|
||||
expression gets fairly complex, in which case the former tends to
|
||||
be easier to read. Again, just do not mix styles in the same part
|
||||
of the code and mimic existing styles in the neighbourhood.
|
||||
|
||||
- When splitting a long logical line, with everything else being
|
||||
equal, it is preferable to split after the operator at higher
|
||||
level in the parse tree. That is, this is more preferable:
|
||||
|
||||
if (a_very_long_variable * that_is_used_in +
|
||||
a_very_long_expression) {
|
||||
...
|
||||
|
||||
than
|
||||
|
||||
if (a_very_long_variable *
|
||||
that_is_used_in + a_very_long_expression) {
|
||||
...
|
||||
|
||||
- Some clever tricks, like using the !! operator with arithmetic
|
||||
constructs, can be extremely confusing to others. Avoid them,
|
||||
unless there is a compelling reason to use them.
|
||||
|
|
Loading…
Reference in a new issue