HiveBrain v1.2.0
Get Started
← Back to all entries
patternbashMinor

Shell script to display environment variables

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptenvironmentshellvariablesdisplay

Problem

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.

Solution

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.

Code Snippets

#!/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
less >/dev/null && PAGER=less
if type less >/dev/null
if type less >/dev/null 2>&1
if [ -z ${1+x} ]; then

Context

StackExchange Code Review Q#133032, answer score: 3

Revisions (0)

No revisions yet.