Skip to content

Commit a378fee

Browse files
matvoregitster
authored andcommitted
Documentation: add shell guidelines
Add the following guideline to Documentation/CodingGuidelines: Break overlong lines after "&&", "||", and "|", not before them; that way the command can continue to subsequent lines without backslash at the end. And the following to t/README (since it is specific to writing tests): Pipes and $(git ...) should be avoided when they swallow exit codes of Git processes Signed-off-by: Matthew DeVore <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 441ee35 commit a378fee

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

Documentation/CodingGuidelines

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,24 @@ For shell scripts specifically (not exhaustive):
118118
do this
119119
fi
120120

121+
- If a command sequence joined with && or || or | spans multiple
122+
lines, put each command on a separate line and put && and || and |
123+
operators at the end of each line, rather than the start. This
124+
means you don't need to use \ to join lines, since the above
125+
operators imply the sequence isn't finished.
126+
127+
(incorrect)
128+
grep blob verify_pack_result \
129+
| awk -f print_1.awk \
130+
| sort >actual &&
131+
...
132+
133+
(correct)
134+
grep blob verify_pack_result |
135+
awk -f print_1.awk |
136+
sort >actual &&
137+
...
138+
121139
- We prefer "test" over "[ ... ]".
122140

123141
- We do not write the noiseword "function" in front of shell

t/README

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -474,6 +474,33 @@ And here are the "don'ts:"
474474
platform commands; just use '! cmd'. We are not in the business
475475
of verifying that the world given to us sanely works.
476476

477+
- Don't feed the output of a git command to a pipe, as in:
478+
479+
git -C repo ls-files |
480+
xargs -n 1 basename |
481+
grep foo
482+
483+
which will discard git's exit code and may mask a crash. In the
484+
above example, all exit codes are ignored except grep's.
485+
486+
Instead, write the output of that command to a temporary
487+
file with ">" or assign it to a variable with "x=$(git ...)" rather
488+
than pipe it.
489+
490+
- Don't use command substitution in a way that discards git's exit
491+
code. When assigning to a variable, the exit code is not discarded,
492+
e.g.:
493+
494+
x=$(git cat-file -p $sha) &&
495+
...
496+
497+
is OK because a crash in "git cat-file" will cause the "&&" chain
498+
to fail, but:
499+
500+
test "refs/heads/foo" = "$(git symbolic-ref HEAD)"
501+
502+
is not OK and a crash in git could go undetected.
503+
477504
- Don't use perl without spelling it as "$PERL_PATH". This is to help
478505
our friends on Windows where the platform Perl often adds CR before
479506
the end of line, and they bundle Git with a version of Perl that

0 commit comments

Comments
 (0)