{"thread":{"id":"29760","subject":"[PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection","startedAt":"2012-02-24T21:12:00Z","lastAt":"2012-02-27T18:56:50Z","messageCount":4,"participants":["Tim Henigan","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"185527","messageId":"1330117921-8257-1-git-send-email-tim.henigan@gmail.com","threadId":"29760","inReplyTo":null,"subject":"[PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-02-24T21:12:00Z","receivedAt":"2012-02-24T21:12:00Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"During code review of some patches, it was noted that redirection operators\nshould have space before, but no space after them.\n\nSigned-off-by: Tim Henigan \n---\n Documentation/CodingGuidelines | 5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex 4830086..a4ffe7c 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -35,6 +35,11 @@ For shell scripts specifically (not exhaustive):\n \n - Case arms are indented at the same depth as case and esac lines.\n \n+ - Redirection operators should be written with space before, but\n+ no space after them. For example:\n+ 'echo test >$file' is preferred over\n+ 'echo test > $file'\n+\n - We prefer $( ... ) for command substitution; unlike ``, it\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\n-- \n1.7.9.1\n"},{"id":"185528","messageId":"1330117921-8257-2-git-send-email-tim.henigan@gmail.com","threadId":"29760","inReplyTo":"1330117921-8257-1-git-send-email-tim.henigan@gmail.com","subject":"[PATCH 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts","fromName":"Tim Henigan","fromEmail":"tim.henigan@gmail.com","sentAt":"2012-02-24T21:12:01Z","receivedAt":"2012-02-24T21:12:01Z","isPatch":true,"sender":{"key":"tim.henigan@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42022?v=4"},"body":"During the code review of a recent patch, it was noted that shell scripts\nmust not use 'which'. The output of the command is not machine parseable\nand its exit code is not reliable across platforms.\n\nSigned-off-by: Tim Henigan \n---\n Documentation/CodingGuidelines | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\nindex a4ffe7c..3505a4b 100644\n--- a/Documentation/CodingGuidelines\n+++ b/Documentation/CodingGuidelines\n@@ -44,6 +44,10 @@ For shell scripts specifically (not exhaustive):\n properly nests. It should have been the way Bourne spelled\n it from day one, but unfortunately isn't.\n \n+ - The use of 'which' is not allowed. The output of 'which' is not\n+ machine parseable and its exit code is not reliable across\n+ platforms.\n+\n - We use POSIX compliant parameter substitutions and avoid bashisms;\n namely:\n \n-- \n1.7.9.1\n"},{"id":"185542","messageId":"7vy5ro85sv.fsf@alter.siamese.dyndns.org","threadId":"29760","inReplyTo":"1330117921-8257-1-git-send-email-tim.henigan@gmail.com","subject":"Re: [PATCH 1/2] CodingGuidelines: Add a note about spaces after redirection","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-27T18:55:28Z","receivedAt":"2012-02-27T18:55:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n> + - Redirection operators should be written with space before, but\n> + no space after them. For example:\n> + 'echo test >$file' is preferred over\n> + 'echo test > $file'\n> +\n\nIf you are using a $file placeholder, then we would need to show readers\nthat they need to be enclosed in dq to prevent some versions of bash from\ngiving us a false warning, and explicitly say why.\n\n $ bash\n bash$ file='/var/tmp/f i l e'\n bash$ >$file\n bash: $file: ambiguous redirect\n bash$ >\"$file\"\n bash$ ls -1 \"$file\"\n /var/tmp/f i l e\n\nAdding something like this after your two line examples, after updating\nthem to use \"$file\" instead, should be sufficient:\n\n\tNote that even though it is not required by POSIX to double quote\n\tthe redirection target in a variable like the above example, our\n\tcode does so because some versions of bash issue an warning unless\n\twe do.\n"},{"id":"185543","messageId":"7vty2c85ql.fsf@alter.siamese.dyndns.org","threadId":"29760","inReplyTo":"1330117921-8257-2-git-send-email-tim.henigan@gmail.com","subject":"Re: [PATCH 2/2] CodingGuidelines: Add note forbidding use of 'which' in shell scripts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-27T18:56:50Z","receivedAt":"2012-02-27T18:56:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tim Henigan writes:\n\n> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines\n> index a4ffe7c..3505a4b 100644\n> --- a/Documentation/CodingGuidelines\n> +++ b/Documentation/CodingGuidelines\n> @@ -44,6 +44,10 @@ For shell scripts specifically (not exhaustive):\n> properly nests. It should have been the way Bourne spelled\n> it from day one, but unfortunately isn't.\n> \n> + - The use of 'which' is not allowed. The output of 'which' is not\n> + machine parseable and its exit code is not reliable across\n> + platforms.\n\nIt is more helpful to say \"If you want to do Z, use X, not Y because Y is\nbroken for such and such reasons\", rather than \"Never use Y because Y is\nbroken ...\".\n\nIn this case, Z is \"find out if a command is available on user's $PATH\",\nand X is \"type\", I think.\n"}]}