0
\$\begingroup\$

I have a bash script that is meant to start a google search from the command line. I am also looking to implement a way of getting source code for the search page possibly through a subscript. This is what I have so far:

if [[ $1 == "-n" ]]; then
    if [[ $# -lt 2 ]]; then
        echo "Usage: google [-n] term [, term [, term...]]"
        exit 1
    fi
fi

if [[ $1 != "-n" ]]; then
    if [[ $# -lt 1 ]]; then
        echo "Usage: google [-n] term [, term [, term...]]"
        exit 2
    fi

    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

search=""

for i in ${BASH_ARGV[*]}; do
    search="$i+$search"
done

escapedSearch=`node -e "a = escape(\"$search\"); console.log(a)"`


open "http://www.google.com/search?q=$escapedSearch"
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
2
\$\begingroup\$
    echo "To use this tool without escapes include -n as the first agument"

Despite what this message seems to suggest, it looks like the script will use node to apply escapes, regardless of the -n parameter. So this message is either poorly phrased or you have a bug.

Also note that if -n was specified, it will become part of the search string. I doubt that's intended, so it looks like a bug.

When there are not enough parameters, separating the cases of whether the first parameter was -n or not, just for the sake of using a different exit code, seems overkill. I suggest to combine those two cases, and use the same exit code for both.

As for handling -n, I suggest that after the check on the first parameter, save the value in a variable, for example no_escape=1, and do a shift. This will help simplifying the test of the argument processing.

Don't use backticks for sub shells. Use the modern $(...) syntax instead.

To check if an executable exists, it's better to use the exit code of running it rather than trying to match the output, for example like this:

if ! node -v > /dev/null; then
    echo node is not available, sorry bud
fi
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.