From d71b46b9c31d6c2aaf67be8ee8f61cd5705031c8 Mon Sep 17 00:00:00 2001 From: koalaman Date: Mon, 26 Dec 2016 14:12:12 -0800 Subject: [PATCH] Updated SC2116 (markdown) --- SC2116.md | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/SC2116.md b/SC2116.md index 281bc28..626f03b 100644 --- a/SC2116.md +++ b/SC2116.md @@ -3,19 +3,47 @@ ### Problematic code: ```sh -a=$(echo $?) +greeting=$(echo "Hello, $name") +# or +tar czf "$(echo "$(date +%F).tar.gz")" * ``` ### Correct code: ```sh -a="$?" +greeting="Hello, $name" +# or +tar czf "$(date +%F).tar.gz" * ``` ### Rationale: -Most of the time, this is an useless echo meaning it isn't doing anything that the Shell can't already do. Having the shell expand the contents for you is simpler and more reliable. Just remember to double quote the argument! +You appear to be using `echo` to write a value to stdout, and then using `$(..)` or `` `..` `` to capture the value again. This is as pointless as mailing yourself a postcard: you already have what you want, so there's no need to send it on a round trip. + +You can just replace `$(echo myvalue)` with `myvalue`. ### Exceptions -None I am aware of at the moment. +Sometimes this pattern is used because of side effect of `echo` or expansions. For example, here `$(echo ..)` is used to expand a glob. +``` +glob="*.png" +files="$(echo $var)" +``` + +The `echo` is not useless, but this code is problematic because it concatenates filenames by spaces. This will break filenames containing spaces and other characters later when the list is split again. Better options are: + +* Arrays, if supported by the shell: `files=( $glob ); echo "The first file is ${files[0]}"` +* Positional parameters when possible: `set -- $glob; echo "The first file is $1"` +* Delaying expansion until it's needed: `for file in $glob; do ...` + +All three methods will let you avoid issues with special characters in filenames. + +As another example, here `$(echo ..)` is used to expand escape sequences: +``` +unexpanded='var\tvalue' +expanded="$(echo "$var")" +``` + +In this case, use `printf` instead. It's well defined with regard to escape sequences. + +Finally, if you really do want to concatenate a series of elements by a character like space, consider doing it explicitly with `for` or `printf` (e.g. `printf '%s\n' $glob`). \ No newline at end of file