123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403404405406407408409410411412413414415416417418419420421422423424425426427428429430431432433434435436437438439440441442443444445446447448449450451452453454455456457458459460461462463464465466467468469470471472473474475476477478479480481482483484485486487488489490491492493494495496497498499500501502503504505506507508509510511512513514515516517518519520521522523524525526527528529530531532533534535536537538539540541542543544545546547548549550551552553554555556557558559560561562563564565566567568569570571572573574575576577578579580581582583584585586587588589590591592593594595596597598599600601602603604605606607608609610611612613614615616617618619620621622623624625626627628629630631632633634635636637638639640641 |
- 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.
|