3
\$\begingroup\$

Goal of Program

Consider a template file template.txt with double brace variables, intended to be replaced by values:

hello there {{ MY_VAR1 }}
some other stuff
some other stuff
foo: {{ MY_VAR2 }}
{{ MY_VAR2 }} is the value of MY_VAR2

and assume you have defined and exported those variables:

export MY_VAR1=val1
export MY_VAR2=val2

we want a script fill_template such that fill_template template.txt produces:

hello there val1
some other stuff
some other stuff
foo: val2
val2 is the value of MY_VAR2

and which gives an appropriate error message if any of the required template variables are not defined.

Code for review

Here is working code for fill_template:

#!/bin/bash

if [[ ! -f $1 ]]; then
  >&2 echo "Usage: $0 <filename>"
  exit 1
fi

# Gather all the required template variables

vars=()
while IFS= read -r line; do
    vars+=( "$line" )
done < <( awk 'match($0, /{{ (.*) }}/, a) { print a[1] }' "$1" | sort -u )

# Verify that all template variables are set and exported

missing=()
for var in "${vars[@]}"; do
  if [[ -z ${!var+x} ]]; then
    missing+=( "$var" )
  fi
done

if [[ ${#missing[@]} -gt 0 ]]; then
  >&2 echo "The following required variables have not been set and exported:"
  for var in "${missing[@]}"; do
    >&2 echo "${var}"
  done
  exit 1
fi

# Dynamically construct the sed cmd to do the replacement

sed_cmd=
for var in "${vars[@]}"; do
  sed_cmd+="s/\\{\\{ *${var} *}}/${!var}/g;"
done

sed -E "${sed_cmd}" "$1"

Notes

  • All comments welcome, from the high-level approach to nitpicks.
  • Currently we assume there will be only one {{ TEMPLATE_VAR }} per line
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Make the template variable name extraction more strict

The Awk command extracts the template variable names using the pattern /{{ (.*) }}/. This leaves some room to human errors. For example excess whitespace, as in {{ MY_VAR }}. This can be especially frustrating for trailing whitespace, which will be invisible when printing the list of missing variables. I think it would be good to strip whitespaces after {{ and before }}.

But that's not quite enough. Consider such line in the input:

hello there {{ MY_VAR1 }} foo bar {{ baz }}

Although using multiple variables is explicitly unsupported, this blows up in the face of the user in a nasty way:

$ MY_VAR1=foo/bar/bazo MY_VAR2=bar bash script.sh input.txt
a.sh: line 19: MY_VAR1 }} foo bar {{ baz: bad substitution
a.sh: line 33: MY_VAR1 }} foo bar {{ baz: bad substitution
hello there {{ MY_VAR1 }} foo bar {{ baz }}
some other stuff
some other stuff
foo: {{ MY_VAR2 }}
{{ MY_VAR2 }} is the value of MY_VAR2

The error messages are unfortunately incomprehensible.

Since the template variable names are expected to take values from shell variables, it would make sense to enforce a stricter pattern.

Even if the script is not intended to handle sophisticated scenarios, I think it should handle such user mistakes more gracefully.

Consistency

The Sed command replacing template variable names with values uses the pattern \\{\\{ *${var} *}}. This is not consistent with the one in the Awk command, because of stripping the whitespace. As mentioned earlier, I would adjust the Awk command to make it consistent.

Error handling

In the example above with a user mistake, the script continued to execute even after the error. To catch such issues and terminate the program early I recommend adding this line at the very beginning:

set -euo pipefail

Beware of some gotchas

/ in the template variable names and values will break the Sed command.

As for the names, a more strict handling as mentioned earlier will prevent this issue.

As for the values, / appearing in the values doesn't sound too crazy, because I can easily imagine wanting to insert path strings. So I think it's a legitimate concern that would be good to address.

Usability

The limitation to one template variable per line seems a bit artificial.

Currently the script fails fast when the user mistakenly tries to use multiple per line, that's a good behavior to preserve. (I'm pointing this out because if you simply enforce more stricter checking on the name patterns, this fail-fast behavior may no longer be the case. And if I had to choose between cryptic failures, and quietly ignored missed template variables, I would prefer cryptic failures.)

\$\endgroup\$
2
  • \$\begingroup\$ Janos, great comments. Tyvm. I'm going to make the suggested changes and then repost the revised version. As a side note, I was curious what you thought of this warning against set -e -- not in relation to my script, but just in general. \$\endgroup\$
    – Jonah
    Commented Mar 31, 2019 at 14:38
  • \$\begingroup\$ @Jonah my personal recommendation is to go ahead and use set -e, but beware of possible gotchas, and add your own error checking too ;-) \$\endgroup\$
    – janos
    Commented Mar 31, 2019 at 14:46

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.