diff options
Diffstat (limited to 'Documentation/CodingGuidelines')
| -rw-r--r-- | Documentation/CodingGuidelines | 307 |
1 files changed, 241 insertions, 66 deletions
diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index 9495df835d..87904791cb 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -1,5 +1,5 @@ -Like other projects, we also have some guidelines to keep to the -code. For Git in general, a few rough rules are: +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." @@ -40,7 +40,7 @@ As for more concrete guidelines, just imitate the existing code 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 +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 @@ -185,8 +185,51 @@ For shell scripts specifically (not exhaustive): - 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 ;-) + hopefully nobody starts using "local" before all shells that matter + support it (notably, ksh from AT&T Research does not support it yet). + + - Some versions of shell do not understand "export variable=value", + so we write "variable=value" and then "export variable" on two + separate lines. + + - Some versions of dash have broken variable assignment when prefixed + with "local", "export", and "readonly", in that the value to be + assigned goes through field splitting at $IFS unless quoted. + + (incorrect) + local variable=$value + local variable=$(command args) + + (correct) + local variable="$value" + local variable="$(command args)" + + - The common construct + + VAR=VAL command args + + to temporarily set and export environment variable VAR only while + "command args" is running is handy, but this triggers an + unspecified behaviour according to POSIX when used for a command + that is not an external command (like shell functions). Indeed, + dash 0.5.10.2-6 on Ubuntu 20.04, /bin/sh on FreeBSD 13, and AT&T + ksh all make a temporary assignment without exporting the variable, + in such a case. As it does not work portably across shells, do not + use this syntax for shell functions. A common workaround is to do + an explicit export in a subshell, like so: + + (incorrect) + VAR=VAL func args + + (correct) + ( + VAR=VAL && + export VAR && + func args + ) + + but be careful that the effect "func" makes to the variables in the + current shell will be lost across the subshell boundary. - Use octal escape sequences (e.g. "\302\242"), not hexadecimal (e.g. "\xc2\xa2") in printf format strings, since hexadecimal escape @@ -198,6 +241,16 @@ For C programs: - We use tabs to indent, and interpret tabs as taking up to 8 spaces. + - Nested C preprocessor directives are indented after the hash by one + space per nesting level. + + #if FOO + # include <foo.h> + # if BAR + # include <bar.h> + # endif + #endif + - We try to keep to at most 80 characters per line. - As a Git developer we assume you have a reasonably modern compiler @@ -205,6 +258,14 @@ For C programs: ensure your patch is clear of all compiler warnings we care about, by e.g. "echo DEVELOPER=1 >>config.mak". + - When using DEVELOPER=1 mode, you may see warnings from the compiler + like "error: unused parameter 'foo' [-Werror=unused-parameter]", + which indicates that a function ignores its argument. If the unused + parameter can't be removed (e.g., because the function is used as a + callback and has to match a certain interface), you can annotate + the individual parameters with the UNUSED (or MAYBE_UNUSED) + keyword, like "int foo UNUSED". + - 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 @@ -218,7 +279,7 @@ For C programs: . 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}; + 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 @@ -250,7 +311,9 @@ For C programs: 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). + the first statement (i.e. -Wdeclaration-after-statement). It is + encouraged to have a blank line between the end of the declarations + and the first statement in the block. - NULL pointers shall be written as NULL, not as 0. @@ -270,6 +333,13 @@ For C programs: while( condition ) func (bar+1); + - A binary operator (other than ",") and ternary conditional "?:" + have a space on each side of the operator to separate it from its + operands. E.g. "A + 1", not "A+1". + + - A unary operator (other than "." and "->") have no space between it + and its operand. E.g. "(char *)ptr", not "(char *) ptr". + - 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: @@ -446,12 +516,41 @@ For C programs: 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. + implementations and sha1dc/, must be <git-compat-util.h>. This + header file insulates other header files and source files from + platform differences, like which system header files must be + included in what order, and what C preprocessor feature macros must + be defined to trigger certain features we expect out of the system. + A collorary to this is that C files should not directly include + system header files themselves. + + There are some exceptions, because certain group of files that + implement an API all have to include the same header file that + defines the API and it is convenient to include <git-compat-util.h> + there. Namely: + + - the implementation of the built-in commands in the "builtin/" + directory that include "builtin.h" for the cmd_foo() prototype + definition, + + - the test helper programs in the "t/helper/" directory that include + "t/helper/test-tool.h" for the cmd__foo() prototype definition, + + - the xdiff implementation in the "xdiff/" directory that includes + "xdiff/xinclude.h" for the xdiff machinery internals, + + - the unit test programs in "t/unit-tests/" directory that include + "t/unit-tests/test-lib.h" that gives them the unit-tests + framework, and + + - the source files that implement reftable in the "reftable/" + directory that include "reftable/system.h" for the reftable + internals, + + are allowed to assume that they do not have to include + <git-compat-util.h> themselves, as it is included as the first + '#include' in these header files. These headers must be the first + header file to be "#include"d in them, though. - A C file must directly include the header files that declare the functions and the types it uses, except for the functions and types @@ -486,11 +585,61 @@ For C programs: use your own debugger and arguments. Example: `GIT_DEBUGGER="ddd --gdb" ./bin-wrappers/git log` (See `wrap-for-bin.sh`.) + - The primary data structure that a subsystem 'S' deals with is called + `struct S`. Functions that operate on `struct S` are named + `S_<verb>()` and should generally receive a pointer to `struct S` as + first parameter. E.g. + + struct strbuf; + + void strbuf_add(struct strbuf *buf, ...); + + void strbuf_reset(struct strbuf *buf); + + is preferred over: + + struct strbuf; + + void add_string(struct strbuf *buf, ...); + + void reset_strbuf(struct strbuf *buf); + + - There are several common idiomatic names for functions performing + specific tasks on a structure `S`: + + - `S_init()` initializes a structure without allocating the + structure itself. + + - `S_release()` releases a structure's contents without freeing the + structure. + + - `S_clear()` is equivalent to `S_release()` followed by `S_init()` + such that the structure is directly usable after clearing it. When + `S_clear()` is provided, `S_init()` shall not allocate resources + that need to be released again. + + - `S_free()` releases a structure's contents and frees the + structure. + + - Function names should be clear and descriptive, accurately reflecting + their purpose or behavior. Arbitrary suffixes that do not add meaningful + context can lead to confusion, particularly for newcomers to the codebase. + + Historically, the '_1' suffix has been used in situations where: + + - A function handles one element among a group that requires similar + processing. + - A recursive function has been separated from its setup phase. + + The '_1' suffix can be used as a concise way to indicate these specific + cases. However, it is recommended to find a more descriptive name wherever + possible to improve the readability and maintainability of the code. + For Perl programs: - Most of the C guidelines above apply. - - We try to support Perl 5.8 and later ("use Perl 5.008"). + - We try to support Perl 5.8.1 and later ("use Perl 5.008001"). - use strict and use warnings are strongly preferred. @@ -518,7 +667,7 @@ For Perl programs: For Python scripts: - - We follow PEP-8 (http://www.python.org/dev/peps/pep-0008/). + - We follow PEP-8 (https://peps.python.org/pep-0008/). - As a minimum, we aim to be compatible with Python 2.7. @@ -578,7 +727,7 @@ Externally Visible Names . 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. `-`), + formed by concatenating the words without punctuation marks (e.g. `-`), and are broken using bumpyCaps in documentation as a hint to the reader. @@ -612,15 +761,15 @@ Writing Documentation: - Prefer succinctness and matter-of-factly describing functionality in the abstract. E.g. - --short:: Emit output in the short-format. + `--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... + `--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 @@ -630,12 +779,12 @@ Writing Documentation: 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 + 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 + 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 @@ -653,18 +802,72 @@ Writing 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>] +Markup: + + Literal parts (e.g. use of command-line options, command names, + branch names, URLs, pathnames (files and directories), configuration and + environment variables) must be typeset as verbatim (i.e. wrapped with + backticks): + `--pretty=oneline` + `git rev-list` + `remote.pushDefault` + `http://git.example.com` + `.git/config` + `GIT_DIR` + `HEAD` + `umask`(2) + + 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` + + Placeholders are spelled in lowercase and enclosed in + angle brackets surrounded by underscores: + _<file>_ + _<commit>_ If a placeholder has multiple words, they are separated by dashes: - <new-branch-name> - --template=<template-directory> + _<new-branch-name>_ + _<template-directory>_ + + When needed, use a distinctive identifier for placeholders, usually + made of a qualification and a type: + _<git-dir>_ + _<key-id>_ + + Git's Asciidoc processor has been tailored to treat backticked text + as complex synopsis. When literal and placeholders are mixed, you can + use the backtick notation which will take care of correctly typesetting + the content. + `--jobs <n>` + `--sort=<key>` + `<directory>/.git` + `remote.<name>.mirror` + `ssh://[<user>@]<host>[:<port>]/<path-to-git-repo>` + +As a side effect, backquoted placeholders are correctly typeset, but +this style is not recommended. + +Synopsis Syntax + + The synopsis (a paragraph with [synopsis] attribute) is automatically + formatted by the toolchain and does not need typesetting. + + A few commented examples follow to provide reference when writing or + modifying command usage strings and synopsis sections in the manual + pages: Possibility of multiple occurrences is indicated by three dots: <file>... @@ -674,6 +877,9 @@ Writing Documentation: [<file>...] (Zero or more of <file>.) + An optional parameter needs to be typeset with unconstrained pairs + [<repository>] + --exec-path[=<path>] (Option with an optional argument. Note that the "=" is inside the brackets.) @@ -697,14 +903,14 @@ Writing Documentation: Don't: --track[=(direct | inherit)] Parentheses are used for grouping: - [(<rev> | <range>)...] + [(<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>) + git remote set-head <name> (-a|-d|<branch>) (One and only one of "-a", "-d" or "<branch>" _must_ (no square brackets) be provided.) @@ -720,37 +926,6 @@ Writing Documentation: 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 |