about summary refs log tree commit diff
path: root/Documentation/CodingGuidelines
diff options
context:
space:
mode:
Diffstat (limited to 'Documentation/CodingGuidelines')
-rw-r--r--Documentation/CodingGuidelines639
1 files changed, 639 insertions, 0 deletions
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
new file mode 100644
index 000000000000..f45db5b72740
--- /dev/null
+++ b/Documentation/CodingGuidelines
@@ -0,0 +1,639 @@
+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 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
+
+ - 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 identifer 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);
+
+ - 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.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,
+ 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.