Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 1 | Like other projects, we also have some guidelines to keep to the |
| 2 | code. For git in general, three rough rules are: |
| 3 | |
| 4 | - Most importantly, we never say "It's in POSIX; we'll happily |
| 5 | ignore your needs should your system not conform to it." |
| 6 | We live in the real world. |
| 7 | |
| 8 | - However, we often say "Let's stay away from that construct, |
| 9 | it's not even in POSIX". |
| 10 | |
| 11 | - In spite of the above two rules, we sometimes say "Although |
| 12 | this is not in POSIX, it (is so convenient | makes the code |
| 13 | much more readable | has other good characteristics) and |
| 14 | practically all the platforms we care about support it, so |
| 15 | let's use it". |
| 16 | |
| 17 | Again, we live in the real world, and it is sometimes a |
| 18 | judgement call, the decision based more on real world |
| 19 | constraints people face than what the paper standard says. |
| 20 | |
| 21 | |
| 22 | As for more concrete guidelines, just imitate the existing code |
| 23 | (this is a good guideline, no matter which project you are |
Nanako Shiraishi | dfb047b | 2009-01-26 17:32:22 +0900 | [diff] [blame] | 24 | contributing to). It is always preferable to match the _local_ |
| 25 | convention. New code added to git suite is expected to match |
| 26 | the overall style of existing code. Modifications to existing |
| 27 | code is expected to match the style the surrounding code already |
| 28 | uses (even if it doesn't match the overall style of existing code). |
| 29 | |
| 30 | But if you must have a list of rules, here they are. |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 31 | |
| 32 | For shell scripts specifically (not exhaustive): |
| 33 | |
Giuseppe Bilotta | f36a4fa | 2010-12-03 17:47:35 +0100 | [diff] [blame] | 34 | - We use tabs for indentation. |
| 35 | |
| 36 | - Case arms are indented at the same depth as case and esac lines. |
| 37 | |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 38 | - We prefer $( ... ) for command substitution; unlike ``, it |
| 39 | properly nests. It should have been the way Bourne spelled |
| 40 | it from day one, but unfortunately isn't. |
| 41 | |
Junio C Hamano | bc97994 | 2010-10-13 11:15:14 -0700 | [diff] [blame] | 42 | - We use POSIX compliant parameter substitutions and avoid bashisms; |
| 43 | namely: |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 44 | |
Junio C Hamano | bc97994 | 2010-10-13 11:15:14 -0700 | [diff] [blame] | 45 | - We use ${parameter-word} and its [-=?+] siblings, and their |
| 46 | colon'ed "unset or null" form. |
| 47 | |
| 48 | - We use ${parameter#word} and its [#%] siblings, and their |
| 49 | doubled "longest matching" form. |
| 50 | |
| 51 | - No "Substring Expansion" ${parameter:offset:length}. |
| 52 | |
| 53 | - No shell arrays. |
| 54 | |
| 55 | - No strlen ${#parameter}. |
| 56 | |
| 57 | - No pattern replacement ${parameter/pattern/string}. |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 58 | |
| 59 | - We use Arithmetic Expansion $(( ... )). |
| 60 | |
Junio C Hamano | 055467d | 2010-09-22 12:15:37 -0700 | [diff] [blame] | 61 | - Inside Arithmetic Expansion, spell shell variables with $ in front |
| 62 | of them, as some shells do not grok $((x)) while accepting $(($x)) |
| 63 | just fine (e.g. dash older than 0.5.4). |
| 64 | |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 65 | - We do not use Process Substitution <(list) or >(list). |
| 66 | |
| 67 | - We prefer "test" over "[ ... ]". |
| 68 | |
| 69 | - We do not write the noiseword "function" in front of shell |
| 70 | functions. |
| 71 | |
Junio C Hamano | 009c98e | 2008-03-01 18:18:16 -0800 | [diff] [blame] | 72 | - As to use of grep, stick to a subset of BRE (namely, no \{m,n\}, |
| 73 | [::], [==], nor [..]) for portability. |
| 74 | |
| 75 | - We do not use \{m,n\}; |
| 76 | |
| 77 | - We do not use -E; |
| 78 | |
| 79 | - We do not use ? nor + (which are \{0,1\} and \{1,\} |
| 80 | respectively in BRE) but that goes without saying as these |
| 81 | are ERE elements not BRE (note that \? and \+ are not even part |
| 82 | of BRE -- making them accessible from BRE is a GNU extension). |
| 83 | |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 84 | For C programs: |
| 85 | |
| 86 | - We use tabs to indent, and interpret tabs as taking up to |
| 87 | 8 spaces. |
| 88 | |
| 89 | - We try to keep to at most 80 characters per line. |
| 90 | |
| 91 | - When declaring pointers, the star sides with the variable |
| 92 | name, i.e. "char *string", not "char* string" or |
| 93 | "char * string". This makes it easier to understand code |
| 94 | like "char *string, c;". |
| 95 | |
| 96 | - We avoid using braces unnecessarily. I.e. |
| 97 | |
| 98 | if (bla) { |
| 99 | x = 1; |
| 100 | } |
| 101 | |
| 102 | is frowned upon. A gray area is when the statement extends |
| 103 | over a few lines, and/or you have a lengthy comment atop of |
| 104 | it. Also, like in the Linux kernel, if there is a long list |
| 105 | of "else if" statements, it can make sense to add braces to |
| 106 | single line blocks. |
| 107 | |
Miklos Vajna | 0b0b8cd | 2008-05-23 01:26:09 +0200 | [diff] [blame] | 108 | - We try to avoid assignments inside if(). |
| 109 | |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 110 | - Try to make your code understandable. You may put comments |
| 111 | in, but comments invariably tend to stale out when the code |
| 112 | they were describing changes. Often splitting a function |
| 113 | into two makes the intention of the code much clearer. |
| 114 | |
| 115 | - Double negation is often harder to understand than no negation |
| 116 | at all. |
| 117 | |
| 118 | - Some clever tricks, like using the !! operator with arithmetic |
| 119 | constructs, can be extremely confusing to others. Avoid them, |
| 120 | unless there is a compelling reason to use them. |
| 121 | |
| 122 | - Use the API. No, really. We have a strbuf (variable length |
| 123 | string), several arrays with the ALLOC_GROW() macro, a |
Johannes Schindelin | c455c87 | 2008-07-21 19:03:49 +0100 | [diff] [blame] | 124 | string_list for sorted string lists, a hash map (mapping struct |
Johannes Schindelin | 6d0618a | 2007-11-08 00:33:19 +0000 | [diff] [blame] | 125 | objects) named "struct decorate", amongst other things. |
| 126 | |
| 127 | - When you come up with an API, document it. |
| 128 | |
| 129 | - The first #include in C files, except in platform specific |
| 130 | compat/ implementations, should be git-compat-util.h or another |
| 131 | header file that includes it, such as cache.h or builtin.h. |
| 132 | |
| 133 | - If you are planning a new command, consider writing it in shell |
| 134 | or perl first, so that changes in semantics can be easily |
| 135 | changed and discussed. Many git commands started out like |
| 136 | that, and a few are still scripts. |
| 137 | |
| 138 | - Avoid introducing a new dependency into git. This means you |
| 139 | usually should stay away from scripting languages not already |
| 140 | used in the git core command set (unless your command is clearly |
| 141 | separate from it, such as an importer to convert random-scm-X |
| 142 | repositories to git). |
Kjetil Barvik | 5719989 | 2009-02-09 21:54:06 +0100 | [diff] [blame] | 143 | |
| 144 | - When we pass <string, length> pair to functions, we should try to |
| 145 | pass them in that order. |
Štěpán Němec | c455bd8 | 2010-11-04 18:12:48 +0100 | [diff] [blame] | 146 | |
| 147 | Writing Documentation: |
| 148 | |
| 149 | Every user-visible change should be reflected in the documentation. |
| 150 | The same general rule as for code applies -- imitate the existing |
| 151 | conventions. A few commented examples follow to provide reference |
| 152 | when writing or modifying command usage strings and synopsis sections |
| 153 | in the manual pages: |
| 154 | |
| 155 | Placeholders are enclosed in angle brackets: |
| 156 | <file> |
| 157 | --sort=<key> |
| 158 | --abbrev[=<n>] |
| 159 | |
Ralf Wildenhues | 469bfc9 | 2011-01-03 20:03:34 +0100 | [diff] [blame^] | 160 | Possibility of multiple occurrences is indicated by three dots: |
Štěpán Němec | c455bd8 | 2010-11-04 18:12:48 +0100 | [diff] [blame] | 161 | <file>... |
| 162 | (One or more of <file>.) |
| 163 | |
| 164 | Optional parts are enclosed in square brackets: |
| 165 | [<extra>] |
| 166 | (Zero or one <extra>.) |
| 167 | |
| 168 | --exec-path[=<path>] |
| 169 | (Option with an optional argument. Note that the "=" is inside the |
| 170 | brackets.) |
| 171 | |
| 172 | [<patch>...] |
| 173 | (Zero or more of <patch>. Note that the dots are inside, not |
| 174 | outside the brackets.) |
| 175 | |
| 176 | Multiple alternatives are indicated with vertical bar: |
| 177 | [-q | --quiet] |
| 178 | [--utf8 | --no-utf8] |
| 179 | |
| 180 | Parentheses are used for grouping: |
| 181 | [(<rev>|<range>)...] |
| 182 | (Any number of either <rev> or <range>. Parens are needed to make |
| 183 | it clear that "..." pertains to both <rev> and <range>.) |
| 184 | |
| 185 | [(-p <parent>)...] |
| 186 | (Any number of option -p, each with one <parent> argument.) |
| 187 | |
| 188 | git remote set-head <name> (-a | -d | <branch>) |
| 189 | (One and only one of "-a", "-d" or "<branch>" _must_ (no square |
| 190 | brackets) be provided.) |
| 191 | |
| 192 | And a somewhat more contrived example: |
| 193 | --diff-filter=[(A|C|D|M|R|T|U|X|B)...[*]] |
| 194 | Here "=" is outside the brackets, because "--diff-filter=" is a |
| 195 | valid usage. "*" has its own pair of brackets, because it can |
| 196 | (optionally) be specified only when one or more of the letters is |
| 197 | also provided. |