[PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection

[PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection

[Tim Henigan]

During code review of some patches, it was noted that redirection operators
should have space before, but no space after them.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
 Documentation/CodingGuidelines |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 4830086..a4ffe7c 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -35,6 +35,11 @@ For shell scripts specifically (not exhaustive):
 
  - Case arms are indented at the same depth as case and esac lines.
 
+ - Redirection operators should be written with space before, but
+   no space after them.  For example:
+      'echo test >$file'  is preferred over
+      'echo test > $file'
+
  - We prefer $( ... ) for command substitution; unlike ``, it
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
-- 
1.7.9.1

[PATCH 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts

[Tim Henigan]

During the code review of a recent patch, it was noted that shell scripts
must not use 'which'.  The output of the command is not machine parseable
and its exit code is not reliable across platforms.

Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
---
 Documentation/CodingGuidelines |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index a4ffe7c..3505a4b 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -44,6 +44,10 @@ For shell scripts specifically (not exhaustive):
    properly nests.  It should have been the way Bourne spelled
    it from day one, but unfortunately isn't.
 
+ - The use of 'which' is not allowed.  The output of 'which' is not
+   machine parseable and its exit code is not reliable across
+   platforms.
+
  - We use POSIX compliant parameter substitutions and avoid bashisms;
    namely:
 
-- 
1.7.9.1

Re: [PATCH 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts

[Junio C Hamano]

Tim Henigan <tim.henigan@gmail.com> writes:

> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> index a4ffe7c..3505a4b 100644
> --- a/Documentation/CodingGuidelines
> +++ b/Documentation/CodingGuidelines
> @@ -44,6 +44,10 @@ For shell scripts specifically (not exhaustive):
>     properly nests.  It should have been the way Bourne spelled
>     it from day one, but unfortunately isn't.
>  
> + - The use of 'which' is not allowed.  The output of 'which' is not
> +   machine parseable and its exit code is not reliable across
> +   platforms.

It is more helpful to say "If you want to do Z, use X, not Y because Y is
broken for such and such reasons", rather than "Never use Y because Y is
broken ...".

In this case, Z is "find out if a command is available on user's $PATH",
and X is "type", I think.

Re: [PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection

[Junio C Hamano]

Tim Henigan <tim.henigan@gmail.com> writes:

> + - Redirection operators should be written with space before, but
> +   no space after them.  For example:
> +      'echo test >$file'  is preferred over
> +      'echo test > $file'
> +

If you are using a $file placeholder, then we would need to show readers
that they need to be enclosed in dq to prevent some versions of bash from
giving us a false warning, and explicitly say why.

  $ bash
  bash$ file='/var/tmp/f i l e'
  bash$ >$file
  bash: $file: ambiguous redirect
  bash$ >"$file"
  bash$ ls -1 "$file"
  /var/tmp/f i l e

Adding something like this after your two line examples, after updating
them to use "$file" instead, should be sufficient:

	Note that even though it is not required by POSIX to double quote
	the redirection target in a variable like the above example, our
	code does so because some versions of bash issue an warning unless
	we do.