]>
Commit | Line | Data |
---|---|---|
6d0618a8 | 1 | Like other projects, we also have some guidelines to keep to the |
2de9b711 | 2 | code. For Git in general, three rough rules are: |
6d0618a8 JS |
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 | ||
dd30800b JH |
21 | - Fixing style violations while working on a real change as a |
22 | preparatory clean-up step is good, but otherwise avoid useless code | |
23 | churn for the sake of conforming to the style. | |
24 | ||
25 | "Once it _is_ in the tree, it's not really worth the patch noise to | |
26 | go and fix it up." | |
27 | Cf. http://article.gmane.org/gmane.linux.kernel/943020 | |
28 | ||
c5e366b1 | 29 | Make your code readable and sensible, and don't try to be clever. |
6d0618a8 JS |
30 | |
31 | As for more concrete guidelines, just imitate the existing code | |
32 | (this is a good guideline, no matter which project you are | |
dfb047b9 | 33 | contributing to). It is always preferable to match the _local_ |
2de9b711 | 34 | convention. New code added to Git suite is expected to match |
dfb047b9 NS |
35 | the overall style of existing code. Modifications to existing |
36 | code is expected to match the style the surrounding code already | |
37 | uses (even if it doesn't match the overall style of existing code). | |
38 | ||
39 | But if you must have a list of rules, here they are. | |
6d0618a8 JS |
40 | |
41 | For shell scripts specifically (not exhaustive): | |
42 | ||
f36a4fa8 GB |
43 | - We use tabs for indentation. |
44 | ||
79fc3ca1 JH |
45 | - Case arms are indented at the same depth as case and esac lines, |
46 | like this: | |
47 | ||
48 | case "$variable" in | |
49 | pattern1) | |
50 | do this | |
51 | ;; | |
52 | pattern2) | |
53 | do that | |
54 | ;; | |
55 | esac | |
f36a4fa8 | 56 | |
48f359bf TH |
57 | - Redirection operators should be written with space before, but no |
58 | space after them. In other words, write 'echo test >"$file"' | |
59 | instead of 'echo test> $file' or 'echo test > $file'. Note that | |
60 | even though it is not required by POSIX to double-quote the | |
61 | redirection target in a variable (as shown above), our code does so | |
62 | because some versions of bash issue a warning without the quotes. | |
63 | ||
6d0618a8 JS |
64 | - We prefer $( ... ) for command substitution; unlike ``, it |
65 | properly nests. It should have been the way Bourne spelled | |
66 | it from day one, but unfortunately isn't. | |
67 | ||
860f70f9 TH |
68 | - If you want to find out if a command is available on the user's |
69 | $PATH, you should use 'type <command>', instead of 'which <command>'. | |
70 | The output of 'which' is not machine parseable and its exit code | |
71 | is not reliable across platforms. | |
72 | ||
bc979945 JH |
73 | - We use POSIX compliant parameter substitutions and avoid bashisms; |
74 | namely: | |
6d0618a8 | 75 | |
bc979945 JH |
76 | - We use ${parameter-word} and its [-=?+] siblings, and their |
77 | colon'ed "unset or null" form. | |
6d0618a8 | 78 | |
bc979945 JH |
79 | - We use ${parameter#word} and its [#%] siblings, and their |
80 | doubled "longest matching" form. | |
6d0618a8 | 81 | |
bc979945 | 82 | - No "Substring Expansion" ${parameter:offset:length}. |
055467dd | 83 | |
bc979945 | 84 | - No shell arrays. |
6d0618a8 | 85 | |
bc979945 | 86 | - No strlen ${#parameter}. |
6d0618a8 | 87 | |
bc979945 | 88 | - No pattern replacement ${parameter/pattern/string}. |
6d0618a8 | 89 | |
bc979945 JH |
90 | - We use Arithmetic Expansion $(( ... )). |
91 | ||
92 | - Inside Arithmetic Expansion, spell shell variables with $ in front | |
93 | of them, as some shells do not grok $((x)) while accepting $(($x)) | |
94 | just fine (e.g. dash older than 0.5.4). | |
6d0618a8 JS |
95 | |
96 | - We do not use Process Substitution <(list) or >(list). | |
97 | ||
03b05c7d HV |
98 | - Do not write control structures on a single line with semicolon. |
99 | "then" should be on the next line for if statements, and "do" | |
100 | should be on the next line for "while" and "for". | |
101 | ||
6d0618a8 JS |
102 | - We prefer "test" over "[ ... ]". |
103 | ||
104 | - We do not write the noiseword "function" in front of shell | |
105 | functions. | |
106 | ||
03b05c7d HV |
107 | - We prefer a space between the function name and the parentheses. The |
108 | opening "{" should also be on the same line. | |
109 | E.g.: my_function () { | |
110 | ||
009c98ee | 111 | - As to use of grep, stick to a subset of BRE (namely, no \{m,n\}, |
a58088ab | 112 | [::], [==], or [..]) for portability. |
009c98ee JH |
113 | |
114 | - We do not use \{m,n\}; | |
115 | ||
116 | - We do not use -E; | |
117 | ||
a58088ab | 118 | - We do not use ? or + (which are \{0,1\} and \{1,\} |
009c98ee JH |
119 | respectively in BRE) but that goes without saying as these |
120 | are ERE elements not BRE (note that \? and \+ are not even part | |
121 | of BRE -- making them accessible from BRE is a GNU extension). | |
122 | ||
5e9637c6 ÆAB |
123 | - Use Git's gettext wrappers in git-sh-i18n to make the user |
124 | interface translatable. See "Marking strings for translation" in | |
125 | po/README. | |
126 | ||
6d0618a8 JS |
127 | For C programs: |
128 | ||
129 | - We use tabs to indent, and interpret tabs as taking up to | |
130 | 8 spaces. | |
131 | ||
132 | - We try to keep to at most 80 characters per line. | |
133 | ||
2de9b711 | 134 | - We try to support a wide range of C compilers to compile Git with, |
a26fd033 AS |
135 | including old ones. That means that you should not use C99 |
136 | initializers, even if a lot of compilers grok it. | |
137 | ||
138 | - Variables have to be declared at the beginning of the block. | |
139 | ||
140 | - NULL pointers shall be written as NULL, not as 0. | |
141 | ||
6d0618a8 JS |
142 | - When declaring pointers, the star sides with the variable |
143 | name, i.e. "char *string", not "char* string" or | |
144 | "char * string". This makes it easier to understand code | |
145 | like "char *string, c;". | |
146 | ||
f57b6cfd JK |
147 | - Use whitespace around operators and keywords, but not inside |
148 | parentheses and not around functions. So: | |
149 | ||
150 | while (condition) | |
151 | func(bar + 1); | |
152 | ||
153 | and not: | |
154 | ||
155 | while( condition ) | |
156 | func (bar+1); | |
157 | ||
6d0618a8 JS |
158 | - We avoid using braces unnecessarily. I.e. |
159 | ||
160 | if (bla) { | |
161 | x = 1; | |
162 | } | |
163 | ||
164 | is frowned upon. A gray area is when the statement extends | |
165 | over a few lines, and/or you have a lengthy comment atop of | |
166 | it. Also, like in the Linux kernel, if there is a long list | |
167 | of "else if" statements, it can make sense to add braces to | |
168 | single line blocks. | |
169 | ||
0b0b8cd7 MV |
170 | - We try to avoid assignments inside if(). |
171 | ||
6d0618a8 JS |
172 | - Try to make your code understandable. You may put comments |
173 | in, but comments invariably tend to stale out when the code | |
174 | they were describing changes. Often splitting a function | |
175 | into two makes the intention of the code much clearer. | |
176 | ||
b75a6ca7 | 177 | - Multi-line comments include their delimiters on separate lines from |
178 | the text. E.g. | |
179 | ||
180 | /* | |
181 | * A very long | |
182 | * multi-line comment. | |
183 | */ | |
184 | ||
cbcfd4e3 JH |
185 | Note however that a comment that explains a translatable string to |
186 | translators uses a convention of starting with a magic token | |
187 | "TRANSLATORS: " immediately after the opening delimiter, even when | |
188 | it spans multiple lines. We do not add an asterisk at the beginning | |
189 | of each line, either. E.g. | |
190 | ||
191 | /* TRANSLATORS: here is a comment that explains the string | |
192 | to be translated, that follows immediately after it */ | |
193 | _("Here is a translatable string explained by the above."); | |
194 | ||
6d0618a8 JS |
195 | - Double negation is often harder to understand than no negation |
196 | at all. | |
197 | ||
198 | - Some clever tricks, like using the !! operator with arithmetic | |
199 | constructs, can be extremely confusing to others. Avoid them, | |
200 | unless there is a compelling reason to use them. | |
201 | ||
202 | - Use the API. No, really. We have a strbuf (variable length | |
203 | string), several arrays with the ALLOC_GROW() macro, a | |
c455c87c | 204 | string_list for sorted string lists, a hash map (mapping struct |
6d0618a8 JS |
205 | objects) named "struct decorate", amongst other things. |
206 | ||
207 | - When you come up with an API, document it. | |
208 | ||
209 | - The first #include in C files, except in platform specific | |
210 | compat/ implementations, should be git-compat-util.h or another | |
211 | header file that includes it, such as cache.h or builtin.h. | |
212 | ||
213 | - If you are planning a new command, consider writing it in shell | |
214 | or perl first, so that changes in semantics can be easily | |
2de9b711 | 215 | changed and discussed. Many Git commands started out like |
6d0618a8 JS |
216 | that, and a few are still scripts. |
217 | ||
2de9b711 | 218 | - Avoid introducing a new dependency into Git. This means you |
6d0618a8 | 219 | usually should stay away from scripting languages not already |
2de9b711 | 220 | used in the Git core command set (unless your command is clearly |
6d0618a8 | 221 | separate from it, such as an importer to convert random-scm-X |
2de9b711 | 222 | repositories to Git). |
57199892 KB |
223 | |
224 | - When we pass <string, length> pair to functions, we should try to | |
225 | pass them in that order. | |
c455bd89 | 226 | |
5e9637c6 ÆAB |
227 | - Use Git's gettext wrappers to make the user interface |
228 | translatable. See "Marking strings for translation" in po/README. | |
229 | ||
c5e366b1 TZ |
230 | For Perl programs: |
231 | ||
232 | - Most of the C guidelines above apply. | |
233 | ||
234 | - We try to support Perl 5.8 and later ("use Perl 5.008"). | |
235 | ||
236 | - use strict and use warnings are strongly preferred. | |
237 | ||
238 | - Don't overuse statement modifiers unless using them makes the | |
239 | result easier to follow. | |
240 | ||
241 | ... do something ... | |
242 | do_this() unless (condition); | |
243 | ... do something else ... | |
244 | ||
245 | is more readable than: | |
246 | ||
247 | ... do something ... | |
248 | unless (condition) { | |
249 | do_this(); | |
250 | } | |
251 | ... do something else ... | |
252 | ||
253 | *only* when the condition is so rare that do_this() will be almost | |
254 | always called. | |
255 | ||
256 | - We try to avoid assignments inside "if ()" conditions. | |
257 | ||
258 | - Learn and use Git.pm if you need that functionality. | |
259 | ||
260 | - For Emacs, it's useful to put the following in | |
261 | GIT_CHECKOUT/.dir-locals.el, assuming you use cperl-mode: | |
262 | ||
263 | ;; note the first part is useful for C editing, too | |
264 | ((nil . ((indent-tabs-mode . t) | |
265 | (tab-width . 8) | |
266 | (fill-column . 80))) | |
267 | (cperl-mode . ((cperl-indent-level . 8) | |
268 | (cperl-extra-newline-before-brace . nil) | |
269 | (cperl-merge-trailing-else . t)))) | |
270 | ||
9ef43dd7 JK |
271 | For Python scripts: |
272 | ||
273 | - We follow PEP-8 (http://www.python.org/dev/peps/pep-0008/). | |
274 | ||
275 | - As a minimum, we aim to be compatible with Python 2.6 and 2.7. | |
276 | ||
277 | - Where required libraries do not restrict us to Python 2, we try to | |
278 | also be compatible with Python 3.1 and later. | |
279 | ||
280 | - When you must differentiate between Unicode literals and byte string | |
281 | literals, it is OK to use the 'b' prefix. Even though the Python | |
282 | documentation for version 2.6 does not mention this prefix, it has | |
283 | been supported since version 2.6.0. | |
284 | ||
c455bd89 ŠN |
285 | Writing Documentation: |
286 | ||
48bc1755 DW |
287 | Most (if not all) of the documentation pages are written in the |
288 | AsciiDoc format in *.txt files (e.g. Documentation/git.txt), and | |
289 | processed into HTML and manpages (e.g. git.html and git.1 in the | |
290 | same directory). | |
bb9f2aec | 291 | |
42e0fae9 MB |
292 | The documentation liberally mixes US and UK English (en_US/UK) |
293 | norms for spelling and grammar, which is somewhat unfortunate. | |
294 | In an ideal world, it would have been better if it consistently | |
295 | used only one and not the other, and we would have picked en_US | |
296 | (if you wish to correct the English of some of the existing | |
297 | documentation, please see the documentation-related advice in the | |
298 | Documentation/SubmittingPatches file). | |
299 | ||
c455bd89 ŠN |
300 | Every user-visible change should be reflected in the documentation. |
301 | The same general rule as for code applies -- imitate the existing | |
ca03c368 JSJ |
302 | conventions. |
303 | ||
304 | A few commented examples follow to provide reference when writing or | |
305 | modifying command usage strings and synopsis sections in the manual | |
306 | pages: | |
c455bd89 | 307 | |
b1afe49d | 308 | Placeholders are spelled in lowercase and enclosed in angle brackets: |
c455bd89 ŠN |
309 | <file> |
310 | --sort=<key> | |
311 | --abbrev[=<n>] | |
312 | ||
469bfc96 | 313 | Possibility of multiple occurrences is indicated by three dots: |
c455bd89 ŠN |
314 | <file>... |
315 | (One or more of <file>.) | |
316 | ||
317 | Optional parts are enclosed in square brackets: | |
318 | [<extra>] | |
319 | (Zero or one <extra>.) | |
320 | ||
321 | --exec-path[=<path>] | |
322 | (Option with an optional argument. Note that the "=" is inside the | |
323 | brackets.) | |
324 | ||
325 | [<patch>...] | |
326 | (Zero or more of <patch>. Note that the dots are inside, not | |
327 | outside the brackets.) | |
328 | ||
329 | Multiple alternatives are indicated with vertical bar: | |
330 | [-q | --quiet] | |
331 | [--utf8 | --no-utf8] | |
332 | ||
333 | Parentheses are used for grouping: | |
334 | [(<rev>|<range>)...] | |
335 | (Any number of either <rev> or <range>. Parens are needed to make | |
336 | it clear that "..." pertains to both <rev> and <range>.) | |
337 | ||
338 | [(-p <parent>)...] | |
339 | (Any number of option -p, each with one <parent> argument.) | |
340 | ||
341 | git remote set-head <name> (-a | -d | <branch>) | |
342 | (One and only one of "-a", "-d" or "<branch>" _must_ (no square | |
343 | brackets) be provided.) | |
344 | ||
345 | And a somewhat more contrived example: | |
346 | --diff-filter=[(A|C|D|M|R|T|U|X|B)...[*]] | |
347 | Here "=" is outside the brackets, because "--diff-filter=" is a | |
348 | valid usage. "*" has its own pair of brackets, because it can | |
349 | (optionally) be specified only when one or more of the letters is | |
350 | also provided. | |
48a8c26c TA |
351 | |
352 | A note on notation: | |
353 | Use 'git' (all lowercase) when talking about commands i.e. something | |
354 | the user would type into a shell and use 'Git' (uppercase first letter) | |
355 | when talking about the version control system and its properties. | |
ca03c368 JSJ |
356 | |
357 | A few commented examples follow to provide reference when writing or | |
358 | modifying paragraphs or option/command explanations that contain options | |
359 | or commands: | |
360 | ||
361 | Literal examples (e.g. use of command-line options, command names, and | |
362 | configuration variables) are typeset in monospace, and if you can use | |
363 | `backticks around word phrases`, do so. | |
364 | `--pretty=oneline` | |
365 | `git rev-list` | |
366 | `remote.pushdefault` | |
367 | ||
368 | Word phrases enclosed in `backtick characters` are rendered literally | |
369 | and will not be further expanded. The use of `backticks` to achieve the | |
370 | previous rule means that literal examples should not use AsciiDoc | |
371 | escapes. | |
372 | Correct: | |
373 | `--pretty=oneline` | |
374 | Incorrect: | |
375 | `\--pretty=oneline` | |
376 | ||
377 | If some place in the documentation needs to typeset a command usage | |
378 | example with inline substitutions, it is fine to use +monospaced and | |
379 | inline substituted text+ instead of `monospaced literal text`, and with | |
380 | the former, the part that should not get substituted must be | |
381 | quoted/escaped. |