mirror of
https://github.com/git/git.git
synced 2024-10-31 14:27:54 +01:00
e8281f0164
* jg/cguide-we-cannot-count: CodingGuidelines: update 'rough' rule count
544 lines
18 KiB
Text
544 lines
18 KiB
Text
Like other projects, we also have some guidelines to keep to the
|
|
code. For Git in general, a few rough rules are:
|
|
|
|
- Most importantly, we never say "It's in POSIX; we'll happily
|
|
ignore your needs should your system not conform to it."
|
|
We live in the real world.
|
|
|
|
- However, we often say "Let's stay away from that construct,
|
|
it's not even in POSIX".
|
|
|
|
- In spite of the above two rules, we sometimes say "Although
|
|
this is not in POSIX, it (is so convenient | makes the code
|
|
much more readable | has other good characteristics) and
|
|
practically all the platforms we care about support it, so
|
|
let's use it".
|
|
|
|
Again, we live in the real world, and it is sometimes a
|
|
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
|
|
(this is a good guideline, no matter which project you are
|
|
contributing to). It is always preferable to match the _local_
|
|
convention. New code added to Git suite is expected to match
|
|
the overall style of existing code. Modifications to existing
|
|
code is expected to match the style the surrounding code already
|
|
uses (even if it doesn't match the overall style of existing code).
|
|
|
|
But if you must have a list of rules, here they are.
|
|
|
|
For shell scripts specifically (not exhaustive):
|
|
|
|
- We use tabs for indentation.
|
|
|
|
- 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"'
|
|
instead of 'echo test> $file' or 'echo test > $file'. Note that
|
|
even though it is not required by POSIX to double-quote the
|
|
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.
|
|
|
|
- If you want to find out if a command is available on the user's
|
|
$PATH, you should use 'type <command>', instead of 'which <command>'.
|
|
The output of 'which' is not machine parseable and its exit code
|
|
is not reliable across platforms.
|
|
|
|
- We use POSIX compliant parameter substitutions and avoid bashisms;
|
|
namely:
|
|
|
|
- We use ${parameter-word} and its [-=?+] siblings, and their
|
|
colon'ed "unset or null" form.
|
|
|
|
- We use ${parameter#word} and its [#%] siblings, and their
|
|
doubled "longest matching" form.
|
|
|
|
- No "Substring Expansion" ${parameter:offset:length}.
|
|
|
|
- No shell arrays.
|
|
|
|
- No strlen ${#parameter}.
|
|
|
|
- No pattern replacement ${parameter/pattern/string}.
|
|
|
|
- We use Arithmetic Expansion $(( ... )).
|
|
|
|
- Inside Arithmetic Expansion, spell shell variables with $ in front
|
|
of them, as some shells do not grok $((x)) while accepting $(($x))
|
|
just fine (e.g. dash older than 0.5.4).
|
|
|
|
- We do not use Process Substitution <(list) or >(list).
|
|
|
|
- Do not write control structures on a single line with semicolon.
|
|
"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,
|
|
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.
|
|
|
|
- We do not use \{m,n\};
|
|
|
|
- We do not use -E;
|
|
|
|
- We do not use ? or + (which are \{0,1\} and \{1,\}
|
|
respectively in BRE) but that goes without saying as these
|
|
are ERE elements not BRE (note that \? and \+ are not even part
|
|
of BRE -- making them accessible from BRE is a GNU extension).
|
|
|
|
- Use Git's gettext wrappers in git-sh-i18n to make the user
|
|
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
|
|
8 spaces.
|
|
|
|
- We try to keep to at most 80 characters per line.
|
|
|
|
- We try to support a wide range of C compilers to compile Git with,
|
|
including old ones. That means that you should not use C99
|
|
initializers, even if a lot of compilers grok it.
|
|
|
|
- Variables have to be declared at the beginning of the block.
|
|
|
|
- NULL pointers shall be written as NULL, not as 0.
|
|
|
|
- When declaring pointers, the star sides with the variable
|
|
name, i.e. "char *string", not "char* string" or
|
|
"char * string". This makes it easier to understand code
|
|
like "char *string, c;".
|
|
|
|
- Use whitespace around operators and keywords, but not inside
|
|
parentheses and not around functions. So:
|
|
|
|
while (condition)
|
|
func(bar + 1);
|
|
|
|
and not:
|
|
|
|
while( condition )
|
|
func (bar+1);
|
|
|
|
- We avoid using braces unnecessarily. I.e.
|
|
|
|
if (bla) {
|
|
x = 1;
|
|
}
|
|
|
|
is frowned upon. A gray area is when the statement extends
|
|
over a few lines, and/or you have a lengthy comment atop of
|
|
it. Also, like in the Linux kernel, if there is a long list
|
|
of "else if" statements, it can make sense to add braces to
|
|
single line blocks.
|
|
|
|
- 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
|
|
they were describing changes. Often splitting a function
|
|
into two makes the intention of the code much clearer.
|
|
|
|
- Multi-line comments include their delimiters on separate lines from
|
|
the text. E.g.
|
|
|
|
/*
|
|
* A very long
|
|
* multi-line comment.
|
|
*/
|
|
|
|
Note however that a comment that explains a translatable string to
|
|
translators uses a convention of starting with a magic token
|
|
"TRANSLATORS: " immediately after the opening delimiter, even when
|
|
it spans multiple lines. We do not add an asterisk at the beginning
|
|
of each line, either. E.g.
|
|
|
|
/* TRANSLATORS: here is a comment that explains the string
|
|
to be translated, that follows immediately after it */
|
|
_("Here is a translatable string explained by the above.");
|
|
|
|
- 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.
|
|
|
|
- Use the API. No, really. We have a strbuf (variable length
|
|
string), several arrays with the ALLOC_GROW() macro, a
|
|
string_list for sorted string lists, a hash map (mapping struct
|
|
objects) named "struct decorate", amongst other things.
|
|
|
|
- When you come up with an API, document it.
|
|
|
|
- The first #include in C files, except in platform specific compat/
|
|
implementations, must be either "git-compat-util.h", "cache.h" or
|
|
"builtin.h". You do not have to include more than one of these.
|
|
|
|
- A C file must directly include the header files that declare the
|
|
functions and the types it uses, except for the functions and types
|
|
that are made available to it by including one of the header files
|
|
it must include by the previous rule.
|
|
|
|
- If you are planning a new command, consider writing it in shell
|
|
or perl first, so that changes in semantics can be easily
|
|
changed and discussed. Many Git commands started out like
|
|
that, and a few are still scripts.
|
|
|
|
- Avoid introducing a new dependency into Git. This means you
|
|
usually should stay away from scripting languages not already
|
|
used in the Git core command set (unless your command is clearly
|
|
separate from it, such as an importer to convert random-scm-X
|
|
repositories to Git).
|
|
|
|
- When we pass <string, length> pair to functions, we should try to
|
|
pass them in that order.
|
|
|
|
- Use Git's gettext wrappers to make the user interface
|
|
translatable. See "Marking strings for translation" in po/README.
|
|
|
|
For Perl programs:
|
|
|
|
- Most of the C guidelines above apply.
|
|
|
|
- We try to support Perl 5.8 and later ("use Perl 5.008").
|
|
|
|
- use strict and use warnings are strongly preferred.
|
|
|
|
- Don't overuse statement modifiers unless using them makes the
|
|
result easier to follow.
|
|
|
|
... do something ...
|
|
do_this() unless (condition);
|
|
... do something else ...
|
|
|
|
is more readable than:
|
|
|
|
... do something ...
|
|
unless (condition) {
|
|
do_this();
|
|
}
|
|
... do something else ...
|
|
|
|
*only* when the condition is so rare that do_this() will be almost
|
|
always called.
|
|
|
|
- We try to avoid assignments inside "if ()" conditions.
|
|
|
|
- Learn and use Git.pm if you need that functionality.
|
|
|
|
- For Emacs, it's useful to put the following in
|
|
GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode:
|
|
|
|
;; note the first part is useful for C editing, too
|
|
((nil . ((indent-tabs-mode . t)
|
|
(tab-width . 8)
|
|
(fill-column . 80)))
|
|
(cperl-mode . ((cperl-indent-level . 8)
|
|
(cperl-extra-newline-before-brace . nil)
|
|
(cperl-merge-trailing-else . t))))
|
|
|
|
For Python scripts:
|
|
|
|
- We follow PEP-8 (http://www.python.org/dev/peps/pep-0008/).
|
|
|
|
- As a minimum, we aim to be compatible with Python 2.6 and 2.7.
|
|
|
|
- Where required libraries do not restrict us to Python 2, we try to
|
|
also be compatible with Python 3.1 and later.
|
|
|
|
- When you must differentiate between Unicode literals and byte string
|
|
literals, it is OK to use the 'b' prefix. Even though the Python
|
|
documentation for version 2.6 does not mention this prefix, it has
|
|
been supported since version 2.6.0.
|
|
|
|
Error Messages
|
|
|
|
- Do not end error messages with a full stop.
|
|
|
|
- Do not capitalize ("unable to open %s", not "Unable to open %s")
|
|
|
|
- Say what the error is first ("cannot open %s", not "%s: cannot open")
|
|
|
|
|
|
Externally Visible Names
|
|
|
|
- For configuration variable names, follow the existing convention:
|
|
|
|
. The section name indicates the affected subsystem.
|
|
|
|
. The subsection name, if any, indicates which of an unbounded set
|
|
of things to set the value for.
|
|
|
|
. The variable name describes the effect of tweaking this knob.
|
|
|
|
The section and variable names that consist of multiple words are
|
|
formed by concatenating the words without punctuations (e.g. `-`),
|
|
and are broken using bumpyCaps in documentation as a hint to the
|
|
reader.
|
|
|
|
When choosing the variable namespace, do not use variable name for
|
|
specifying possibly unbounded set of things, most notably anything
|
|
an end user can freely come up with (e.g. branch names). Instead,
|
|
use subsection names or variable values, like the existing variable
|
|
branch.<name>.description does.
|
|
|
|
|
|
Writing Documentation:
|
|
|
|
Most (if not all) of the documentation pages are written in the
|
|
AsciiDoc format in *.txt files (e.g. Documentation/git.txt), and
|
|
processed into HTML and manpages (e.g. git.html and git.1 in the
|
|
same directory).
|
|
|
|
The documentation liberally mixes US and UK English (en_US/UK)
|
|
norms for spelling and grammar, which is somewhat unfortunate.
|
|
In an ideal world, it would have been better if it consistently
|
|
used only one and not the other, and we would have picked en_US
|
|
(if you wish to correct the English of some of the existing
|
|
documentation, please see the documentation-related advice in the
|
|
Documentation/SubmittingPatches file).
|
|
|
|
Every user-visible change should be reflected in the documentation.
|
|
The same general rule as for code applies -- imitate the existing
|
|
conventions.
|
|
|
|
A few commented examples follow to provide reference when writing or
|
|
modifying command usage strings and synopsis sections in the manual
|
|
pages:
|
|
|
|
Placeholders are spelled in lowercase and enclosed in angle brackets:
|
|
<file>
|
|
--sort=<key>
|
|
--abbrev[=<n>]
|
|
|
|
If a placeholder has multiple words, they are separated by dashes:
|
|
<new-branch-name>
|
|
--template=<template-directory>
|
|
|
|
Possibility of multiple occurrences is indicated by three dots:
|
|
<file>...
|
|
(One or more of <file>.)
|
|
|
|
Optional parts are enclosed in square brackets:
|
|
[<extra>]
|
|
(Zero or one <extra>.)
|
|
|
|
--exec-path[=<path>]
|
|
(Option with an optional argument. Note that the "=" is inside the
|
|
brackets.)
|
|
|
|
[<patch>...]
|
|
(Zero or more of <patch>. Note that the dots are inside, not
|
|
outside the brackets.)
|
|
|
|
Multiple alternatives are indicated with vertical bars:
|
|
[-q | --quiet]
|
|
[--utf8 | --no-utf8]
|
|
|
|
Parentheses are used for grouping:
|
|
[(<rev> | <range>)...]
|
|
(Any number of either <rev> or <range>. Parens are needed to make
|
|
it clear that "..." pertains to both <rev> and <range>.)
|
|
|
|
[(-p <parent>)...]
|
|
(Any number of option -p, each with one <parent> argument.)
|
|
|
|
git remote set-head <name> (-a | -d | <branch>)
|
|
(One and only one of "-a", "-d" or "<branch>" _must_ (no square
|
|
brackets) be provided.)
|
|
|
|
And a somewhat more contrived example:
|
|
--diff-filter=[(A|C|D|M|R|T|U|X|B)...[*]]
|
|
Here "=" is outside the brackets, because "--diff-filter=" is a
|
|
valid usage. "*" has its own pair of brackets, because it can
|
|
(optionally) be specified only when one or more of the letters is
|
|
also provided.
|
|
|
|
A note on notation:
|
|
Use 'git' (all lowercase) when talking about commands i.e. something
|
|
the user would type into a shell and use 'Git' (uppercase first letter)
|
|
when talking about the version control system and its properties.
|
|
|
|
A few commented examples follow to provide reference when writing or
|
|
modifying paragraphs or option/command explanations that contain options
|
|
or commands:
|
|
|
|
Literal examples (e.g. use of command-line options, command names, and
|
|
configuration variables) are typeset in monospace, and if you can use
|
|
`backticks around word phrases`, do so.
|
|
`--pretty=oneline`
|
|
`git rev-list`
|
|
`remote.pushDefault`
|
|
|
|
Word phrases enclosed in `backtick characters` are rendered literally
|
|
and will not be further expanded. The use of `backticks` to achieve the
|
|
previous rule means that literal examples should not use AsciiDoc
|
|
escapes.
|
|
Correct:
|
|
`--pretty=oneline`
|
|
Incorrect:
|
|
`\--pretty=oneline`
|
|
|
|
If some place in the documentation needs to typeset a command usage
|
|
example with inline substitutions, it is fine to use +monospaced and
|
|
inline substituted text+ instead of `monospaced literal text`, and with
|
|
the former, the part that should not get substituted must be
|
|
quoted/escaped.
|