3
\$\begingroup\$

This shell script writes my environment variables. If there is an argument, the shell script will grep for the argument in the environment variables.

PAGER=more
if type less > /dev/null;then PAGER=less; fi
echo $PAGER
if [ -z ${1+x} ]; then printenv|"$PAGER"; else printenv|grep "$1"|"$PAGER"; fi

If the above script is correct then I will work on my shell to be able to parse it. It can parse everything except the last line, which I added today.

\$\endgroup\$
0

1 Answer 1

3
+50
\$\begingroup\$

Readability

Just because Bash let's you cram a lot of statements on a single line doesn't mean you should. I recommend to split this up to multiple lines, and also to put spaces around |, like this:

#!/bin/bash

PAGER=more
if less >/dev/null; then
    PAGER=less
fi
echo $PAGER

if [ -z ${1+x} ]; then
    printenv | "$PAGER"
else
    printenv | grep "$1" | "$PAGER"
fi

But if you like compact writing style, a reasonable compromise is to replace the first if statement with an alternative writing style using && like this:

less >/dev/null && PAGER=less

Error handling

I'm not sure if it's intentional, but if the less command doesn't exist (but in most systems it does), this line will emit an error message:

if type less >/dev/null

To avoid errors, you want to suppress stderr too in addition to stdout:

if type less >/dev/null 2>&1

Prefer simple solutions

This is hacky, cryptic:

if [ -z ${1+x} ]; then

I suggest a much more readable, simple equivalent:

if [ -z "$1" ]; then

Don't repeat yourself

The printenv command is repeated in both branches of the last if statement. You could move it in front of the if, like this:

printenv | \
if [ -z "$1" ]; then
    "$PAGER"
else
    grep "$1" | "$PAGER"
fi

However, in this writing style it's important that the \ on the line before the if is the last character on the line directly in front of the line break.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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