diff options
Diffstat (limited to 'Documentation/CodingGuidelines')
| -rw-r--r-- | Documentation/CodingGuidelines | 640 |
1 files changed, 640 insertions, 0 deletions
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines new file mode 100644 index 0000000..45465bc --- /dev/null +++ b/Documentation/CodingGuidelines @@ -0,0 +1,640 @@ +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. |