threads / patch / 29760

patch, 2 partsCodingGuidelines: Add a note about spaces after redirection

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

## tl;dr

4 messages between Feb 24, 2012 and Feb 27, 2012. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Tim Henigan· Feb 24, 2012, 21:12 UTC · lore

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(-)
Show changes to Documentation/CodingGuidelines +5 −0
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
Tim Henigan· Feb 24, 2012, 21:12 UTC · re: Tim Henigan · lore

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

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(-)
Show changes to Documentation/CodingGuidelines +4 −0
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
Junio C Hamano· Feb 27, 2012, 18:56 UTC · re: Tim Henigan · lore

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

Tim Henigan <tim.henigan@gmail.com> writes:
Show 11 quoted lines
> 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.

Junio C Hamano· Feb 27, 2012, 18:55 UTC · re: Tim Henigan · lore

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

Tim Henigan <tim.henigan@gmail.com> writes:
Show 5 quoted 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'
> +

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.

← back to recent threads