You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
640 lines
21 KiB
640 lines
21 KiB
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://lkml.iu.edu/hypermail/linux/kernel/1001.3/01069.html |
|
|
|
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 parsable 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 pattern replacement ${parameter/pattern/string}. |
|
|
|
- We use Arithmetic Expansion $(( ... )). |
|
|
|
- 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 |
|
|
|
- If a command sequence joined with && or || or | spans multiple |
|
lines, put each command on a separate line and put && and || and | |
|
operators at the end of each line, rather than the start. This |
|
means you don't need to use \ to join lines, since the above |
|
operators imply the sequence isn't finished. |
|
|
|
(incorrect) |
|
grep blob verify_pack_result \ |
|
| awk -f print_1.awk \ |
|
| sort >actual && |
|
... |
|
|
|
(correct) |
|
grep blob verify_pack_result | |
|
awk -f print_1.awk | |
|
sort >actual && |
|
... |
|
|
|
- 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. |
|
|
|
- As a Git developer we assume you have a reasonably modern compiler |
|
and we recommend you to enable the DEVELOPER makefile knob to |
|
ensure your patch is clear of all compiler warnings we care about, |
|
by e.g. "echo DEVELOPER=1 >>config.mak". |
|
|
|
- We try to support a wide range of C compilers to compile Git with, |
|
including old ones. You should not use features from newer C |
|
standard, even if your compiler groks them. |
|
|
|
There are a few exceptions to this guideline: |
|
|
|
. since early 2012 with e1327023ea, we have been using an enum |
|
definition whose last element is followed by a comma. This, like |
|
an array initializer that ends with a trailing comma, can be used |
|
to reduce the patch noise when adding a new identifier at the end. |
|
|
|
. since mid 2017 with cbc0f81d, we have been using designated |
|
initializers for struct (e.g. "struct t v = { .val = 'a' };"). |
|
|
|
. since mid 2017 with 512f41cf, we have been using designated |
|
initializers for array (e.g. "int array[10] = { [5] = 2 }"). |
|
|
|
These used to be forbidden, but we have not heard any breakage |
|
report, and they are assumed to be safe. |
|
|
|
- Variables have to be declared at the beginning of the block, before |
|
the first statement (i.e. -Wdeclaration-after-statement). |
|
|
|
- Declaring a variable in the for loop "for (int i = 0; i < 10; i++)" |
|
is still not allowed in this codebase. |
|
|
|
- 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); |
|
|
|
- Do not explicitly compare an integral value with constant 0 or '\0', |
|
or a pointer value with constant NULL. For instance, to validate that |
|
counted array <ptr, cnt> is initialized but has no elements, write: |
|
|
|
if (!ptr || cnt) |
|
BUG("empty array expected"); |
|
|
|
and not: |
|
|
|
if (ptr == NULL || cnt != 0); |
|
BUG("empty array expected"); |
|
|
|
- We avoid using braces unnecessarily. I.e. |
|
|
|
if (bla) { |
|
x = 1; |
|
} |
|
|
|
is frowned upon. But there are a few exceptions: |
|
|
|
- When the statement extends over a few lines (e.g., a while loop |
|
with an embedded conditional, or a comment). E.g.: |
|
|
|
while (foo) { |
|
if (x) |
|
one(); |
|
else |
|
two(); |
|
} |
|
|
|
if (foo) { |
|
/* |
|
* This one requires some explanation, |
|
* so we're better off with braces to make |
|
* it obvious that the indentation is correct. |
|
*/ |
|
doit(); |
|
} |
|
|
|
- When there are multiple arms to a conditional and some of them |
|
require braces, enclose even a single line block in braces for |
|
consistency. E.g.: |
|
|
|
if (foo) { |
|
doit(); |
|
} else { |
|
one(); |
|
two(); |
|
three(); |
|
} |
|
|
|
- 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: ", 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 its functions and structures |
|
in the header file that exposes the API to its callers. Use what is |
|
in "strbuf.h" as a model for the appropriate tone and level of |
|
detail. |
|
|
|
- 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. |
|
|
|
- Variables and functions local to a given source file should be marked |
|
with "static". Variables that are visible to other source files |
|
must be declared with "extern" in header files. However, function |
|
declarations should not use "extern", as that is already the default. |
|
|
|
- You can launch gdb around your program using the shorthand GIT_DEBUGGER. |
|
Run `GIT_DEBUGGER=1 ./bin-wrappers/git foo` to simply use gdb as is, or |
|
run `GIT_DEBUGGER="<debugger> <debugger-args>" ./bin-wrappers/git foo` to |
|
use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb" |
|
./bin-wrappers/git log` (See `wrap-for-bin.sh`.) |
|
|
|
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.7. |
|
|
|
- Where required libraries do not restrict us to Python 2, we try to |
|
also be compatible with Python 3.1 and later. |
|
|
|
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, |
|
branch names, URLs, pathnames (files and directories), configuration and |
|
environment variables) must be typeset in monospace (i.e. wrapped with |
|
backticks): |
|
`--pretty=oneline` |
|
`git rev-list` |
|
`remote.pushDefault` |
|
`http://git.example.com` |
|
`.git/config` |
|
`GIT_DIR` |
|
`HEAD` |
|
|
|
An environment variable must be prefixed with "$" only when referring to its |
|
value and not when referring to the variable itself, in this case there is |
|
nothing to add except the backticks: |
|
`GIT_DIR` is specified |
|
`$GIT_DIR/hooks/pre-receive` |
|
|
|
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.
|
|
|