+ TODO: Consider giving examples both for good and bad usage.
- + Command substitution on right hand side of assignment should be safe
+ + Command substitution on right hand side of assignment are safe
without quotes.
- + But still, quotes are RECOMMENDED, unless line length is a concern.
-
+ Note that command substitution limits the scope for quotes,
so it is NOT REQUIRED to escape the quotes in deeper levels.
- + TODO: Do we prefer backtics, or "dollar round-bracket"?
+ + Both backtics and "dollar round-bracket" provide command substitution.
+ The folowing rules are RECOMMENDED:
+
+ + For simple constructs, use "dollar round-bracket".
- + Backticks are more readable, especially when there are
- round brackets in the surrounding text.
+ + If there are round brackets in the surrounding text, use backticks,
+ as some editor highlighting logic can get confused.
- + Backticks do not allow nested command substitution.
+ + Avoid nested command substitution.
- + But we might want to avoid nested command substitution anyway?
+ + Put intermediate results into local variables,
+ use "|| die" on each step of command substitution.
+ Code SHOULD NOT be structured in a way where
word splitting is intended.
When was the last time you checked error code of "echo" command,
for example?
+ + Another example: "set -e" in your function has no effect
+ if any ancestor call is done with logical or,
+ for example in "func || code=$?" construct.
+
+ As there is no reliable method of error detection, and there are two
largely independent unreliable methods, the best what we can do
is to apply both. So, code SHOULD explicitly
+ Code MUST to store "$?" and call "set -e" immediatelly afterwards.
+ + Code MUST NOT use this approach when calling functions.
+
+ + That is because functions are instructed to apply "set -e" on their own
+ which (when triggered) will exit the whole entry script.
+
+ + Unless overriden by ERR trap.
+ But code SHOULD NOT set any ERR trap.
+
+ + If code needs exit code of a function, it is RECOMMENDED to use
+ pattern 'code="0"; called_function || code="${?}"'.
+
+ + In this case, contributor MUST make sure nothing in the
+ called_function sub-graph relies on "set -e" behavior,
+ because the call being part of "or construct" disables it.
+
+ Code MAY append "|| true" for benign commands,
when it is clear non-zero exit codes make no difference.
+ + Also in this case, the contributor MUST make sure nothing within
+ the called sub-graph depends on "set -e", as it is disabled.
+
+ Code MUST apply "-u" as unset variable is generally a typo, thus an error.
+ Code MAY temporarily apply "+u" if a command needs that to pass.
+ For most Linux distros, "getopt" is RECOMMENDED.
+Working directory handling
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
++ Functions SHOULD act correctly without neither assuming
+ what the currect working directory is, nor changing it.
+
+ + That is why global variables and arguments SHOULD contain
+ (normalized) full paths.
+
+ + Motivation: Different call sites MAY rely on different working directories.
+
++ A function MAY return (also with nonzero exit code) when working directory
+ is changed.
+
+ + In this case the function documentation MUST clearly state where (and when)
+ is the working directory changed.
+
+ + Exception: Functions with undocumented exit code.
+
+ + Those functions MUST return nonzero code only on "set -e" or "die".
+
+ + Note that both "set -e" and "die" by default result in exit of the whole
+ entry script, but the caller MAY have altered that behavior
+ (by registering ERR trap, or redefining die function).
+
+ + Any callers which use "set +e" or "|| true" MUST make sure
+ their (and their caller ancestors') assumption on working directory
+ are not affected.
+
+ + Such callers SHOULD do that by restoring the original working directory
+ either in their code,
+
+ + or contributors SHOULD do such restoration in the function code,
+ (see below) if that is more convenient.
+
+ + Motivation: Callers MAY rely on this side effect to simplify their logic.
+
++ A function MAY assume a particular directory is already set
+ as the working directory (to save space).
+
+ + In this case function documentation MUST clearly state what the assumed
+ working directory is.
+
+ + Motivation: Callers MAY call several functions with common
+ directory of interest.
+
+ + Example: Several dowload actions to execute in sequence,
+ implemented as functions assuming ${DOWNLOAD_DIR}
+ is the working directory.
+
++ A function MAY change the working directory transiently,
+ before restoring it back before return.
+
+ + Such functions SHOULD use command "pushd" to change the working directory.
+
+ + Such functions SHOULD use "trap 'trap - RETURN; popd' RETURN"
+ imediately after the pushd.
+
+ + In that case, the "trap - RETURN" part MUST be included,
+ to restore any trap set by ancestor.
+
+ + Functions MAY call "trap - RETURN; popd" exlicitly.
+
+ + Such functions MUST NOT call another pushd (before an explicit popd),
+ as traps do not stack within a function.
+
++ If entry scripts also use traps to restore working directory (or other state),
+ they SHOULD use EXIT traps instead.
+
+ + That is because "exit" command, as well as the default behavior
+ of "die" or "set -e" cause direct exit (without skipping function returns).
+
Function size
~~~~~~~~~~~~~
+ Then SHALL be the function definitions, and inside:
- + "set -exuo pipefail" again.
-
- + Following that SHALL be the function documentation explaining API contract.
+ + The body SHALL sart with the function documentation explaining API contract.
Similar to Robot [Documentation] or Python function-level docstring.
+ See below.
- + Following that SHALL be varius TODOs, FIXMEs and code itself.
+ + Following that SHALL be various top-level TODOs and FIXMEs.
+
+ + TODO: Document (in an appropriate place) how TODOs differ from FIXMEs.
+
+ + "set -exuo pipefail" SHALL be the first executable line
+ in the function body, except functions which legitimely need
+ different flags. Those SHALL also start with appropriate "set" command(s).
+
+ + Lines containing code itself SHALL follow.
+ "Code itself" SHALL include comment lines
explaining any non-obvious logic.
- + TODO: Document (in an appropriate place) how TODOs differ from FIXMEs.
-
- + There SHALL be two empty lines before next function definition.
+ + There SHALL be two empty lines between function definitions.
More details on function documentation:
Code Review / csit.git / blobdiff