I'm not quite certain about the use of node and escaping, and I'm going to assume that it does exactly what you need, focusing on the code and style of your script only.
First, I notice that you use an if
statement to check whether $1
is -n, then another where you check whether it is not -n. You could, of course, use an if/then/else construction for that.
Secondly, instead of looping backwards through the command line arguments with ${BASH_ARGV[*]}
, concatenating them with +
es, could have unexpected side effects if you use quoted spaces, as in:
for i in ${BASH_ARGV[*]}; do
search="$i+$search"
done
echo "$search"
Will do something like (I'm going to call the script google.sh
here):
-> % ./google.sh -n this and that "thing at once"
-n+this+and+that+once+at+thing+
Instead, you could use the bash built-in replacement function and the all-parameters variable like this:
args="$*"
search=${args// /+}
echo "$search"
Which would produce:
-> % ./google.sh -n this and that "thing at once"
-n+this+and+that+thing+at+once
Thirdly, I do like "one line ifs", and would change the two checks for missing arguments to -n like follows:
[[ $# -lt 2 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 1
But that is a matter of taste. It does save three lines, though, and makes clear that it is some kind of precondition check (the lines end in exit
).
Lastly, I would always start a script with a shebang, making it executable from the command line directly, but this also depends on how it is going to be used.
Applying these points, the code would look like:
#!/usr/bin/env bash
if [[ $1 == "-n" ]]
then
[[ $# -lt 2 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 1
else
[[ $# -lt 1 ]] && echo "Usage: google [-n] term [, term [, term...]]" && exit 2
if [[ $(node -v) != *"v"* ]]; then
echo "This tool uses node.js to escape url entities"
echo "To use this tool without escapes include -n as the first agument"
echo "(THIS IS NOT RECOMMENDED!)"
exit 3
fi
fi
args="$*"
search=${args// /+}
escapedSearch=`node -e "a = escape(\"$search\"); console.log(a)"`
open "http://www.google.com/search?q=$escapedSearch"
You would probably do something with the -n flag between the then
and else
, like setting a variable, and deciding whether to call node or not, before calling open
.