diff options
Diffstat (limited to 'Documentation/CodingGuidelines')
-rw-r--r-- | Documentation/CodingGuidelines | 758 |
1 files changed, 758 insertions, 0 deletions
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines new file mode 100644 index 0000000..8d3a467 --- /dev/null +++ b/Documentation/CodingGuidelines @@ -0,0 +1,758 @@ +Like other projects, we also have some guidelines for our 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. https://lore.kernel.org/all/20100126160632.3bdbe172.akpm@linux-foundation.org/ + + - Log messages to explain your changes are as important as the + changes themselves. Clearly written code and in-code comments + explain how the code works and what is assumed from the surrounding + context. The log messages explain what the changes wanted to + achieve and why the changes were necessary (more on this in the + accompanying SubmittingPatches document). + +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 are 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 are some language +specific ones. Note that Documentation/ToolsForGit.txt document +has a collection of tips to help you use some external tools +to conform to these guidelines. + +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 ? 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. + + - Even though "local" is not part of POSIX, we make heavy use of it + in our test suite. We do not use it in scripted Porcelains, and + hopefully nobody starts using "local" before they are reimplemented + in C ;-) + + - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g. + "\xc2\xa2") in printf format strings, since hexadecimal escape + sequences are not portable. + + +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. As of Git v2.35.0 Git requires C99 (we check + "__STDC_VERSION__"). You should not use features from a newer C + standard, even if your compiler groks them. + + New C99 features have been phased in gradually, if something's new + in C99 but not used yet don't assume that it's safe to use, some + compilers we target have only partial support for it. These are + considered safe to use: + + . since around 2007 with 2b6854c863a, we have been using + initializer elements which are not computable at load time. E.g.: + + const char *args[] = {"constant", variable, NULL}; + + . 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 }"). + + . since early 2021 with 765dc168882, we have been using variadic + macros, mostly for printf-like trace and debug macros. + + . since late 2021 with 44ba10d6, we have had variables declared in + the for loop "for (int i = 0; i < 10; i++)". + + New C99 features that we cannot use yet: + + . %z and %zu as a printf() argument for a size_t (the %z being for + the POSIX-specific ssize_t). Instead you should use + printf("%"PRIuMAX, (uintmax_t)v). These days the MSVC version we + rely on supports %z, but the C library used by MinGW does not. + + . Shorthand like ".a.b = *c" in struct initializations is known to + trip up an older IBM XLC version, use ".a = { .b = *c }" instead. + See the 33665d98 (reftable: make assignments portable to AIX xlc + v12.01, 2022-03-28). + + - Variables have to be declared at the beginning of the block, before + the first statement (i.e. -Wdeclaration-after-statement). + + - 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 and sha1dc/, must be either "git-compat-util.h" or + one of the approved headers that includes it first for you. (The + approved headers currently include "builtin.h", + "t/helper/test-tool.h", "xdiff/xinclude.h", or + "reftable/system.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 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. + + +Program Output + + We make a distinction between a Git command's primary output and + output which is merely chatty feedback (for instance, status + messages, running transcript, or progress display), as well as error + messages. Roughly speaking, a Git command's primary output is that + which one might want to capture to a file or send down a pipe; its + chatty output should not interfere with these use-cases. + + As such, primary output should be sent to the standard output stream + (stdout), and chatty output should be sent to the standard error + stream (stderr). Examples of commands which produce primary output + include `git log`, `git show`, and `git branch --list` which generate + output on the stdout stream. + + Not all Git commands have primary output; this is often true of + commands whose main function is to perform an action. Some action + commands are silent, whereas others are chatty. An example of a + chatty action commands is `git clone` with its "Cloning into + '<path>'..." and "Checking connectivity..." status messages which it + sends to the stderr stream. + + Error messages from Git commands should always be sent to the stderr + stream. + + +Error Messages + + - Do not end error messages with a full stop. + + - Do not capitalize the first word, only because it is the first word + in the message ("unable to open %s", not "Unable to open %s"). But + "SHA-3 not supported" is fine, because the reason the first word is + capitalized is not because it is at the beginning of the sentence, + but because the word would be spelled in capital letters even when + it appeared in the middle of the sentence. + + - 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). + + In order to ensure the documentation is inclusive, avoid assuming + that an unspecified example person is male or female, and think + twice before using "he", "him", "she", or "her". Here are some + tips to avoid use of gendered pronouns: + + - Prefer succinctness and matter-of-factly describing functionality + in the abstract. E.g. + + --short:: Emit output in the short-format. + + and avoid something like these overly verbose alternatives: + + --short:: Use this to emit output in the short-format. + --short:: You can use this to get output in the short-format. + --short:: A user who prefers shorter output could.... + --short:: Should a person and/or program want shorter output, he + she/they/it can... + + This practice often eliminates the need to involve human actors in + your description, but it is a good practice regardless of the + avoidance of gendered pronouns. + + - When it becomes awkward to stick to this style, prefer "you" when + addressing the hypothetical user, and possibly "we" when + discussing how the program might react to the user. E.g. + + You can use this option instead of --xyz, but we might remove + support for it in future versions. + + while keeping in mind that you can probably be less verbose, e.g. + + Use this instead of --xyz. This option might be removed in future + versions. + + - If you still need to refer to an example person that is + third-person singular, you may resort to "singular they" to avoid + "he/she/him/her", e.g. + + A contributor asks their upstream to pull from them. + + Note that this sounds ungrammatical and unnatural to those who + learned that "they" is only used for third-person plural, e.g. + those who learn English as a second language in some parts of the + world. + + 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: + [<file>...] + (Zero or more of <file>.) + + --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] + + Use spacing around "|" token(s), but not immediately after opening or + before closing a [] or () pair: + Do: [-q | --quiet] + Don't: [-q|--quiet] + + Don't use spacing around "|" tokens when they're used to separate the + alternate arguments of an option: + Do: --track[=(direct|inherit)] + Don't: --track[=(direct | inherit)] + + 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. |