Updating text in a HTML document with Bash

Question

This is a small script to update my project's README file. Notes on improvements from any aspect are welcome!

Here is the document it updates.

readme.sh (Executed like so: ./readme.sh)

#!/usr/bin/env bash

export LANG=en_US.UTF-8
export LANGUAGE=en:el

set -eu

domain_linecount=$(cat black_domain.txt | wc -l)
ipv4_linecount=$(($(cat black_ipv4.txt | wc -l)-$domain_linecount))
ipv6_linecount=$(($(cat black_ipv6.txt | wc -l)-$domain_linecount))

domain_entries=$(printf "%'d" $domain_linecount)
domain_size=$(du -h black_domain.txt | gawk -F'\t' '{ print $1 }')

ipv4_entries=$(printf "%'d" $ipv4_linecount)
ipv4_size=$(du -h black_ipv4.txt | gawk -F'\t' '{ print $1 }')

ipv6_entries=$(printf "%'d" $ipv6_linecount)
ipv6_size=$(du -h black_ipv6.txt | gawk -F'\t' '{ print $1 }')

sed -i \
    -e "s/\(<td id=\"domain-entries\">\).*\(<\/td>\)/<td id=\"domain-entries\">$domain_entries<\/td>/g" \
    -e "s/\(<td id=\"domain-size\">\).*\(<\/td>\)/<td id=\"domain-size\">${domain_size}B<\/td>/g" \
    -e "s/\(<td id=\"ipv4-entries\">\).*\(<\/td>\)/<td id=\"ipv4-entries\">$ipv4_entries<\/td>/g" \
    -e "s/\(<td id=\"ipv4-size\">\).*\(<\/td>\)/<td id=\"ipv4-size\">${ipv4_size}B<\/td>/g" \
    -e "s/\(<td id=\"ipv6-entries\">\).*\(<\/td>\)/<td id=\"ipv6-entries\">$ipv6_entries<\/td>/g" \
    -e "s/\(<td id=\"ipv6-size\">\).*\(<\/td>\)/<td id=\"ipv6-size\">${ipv6_size}B<\/td>/g" ./.github/README.md
# xmlstarlet is probably better to handle this

Answer 1

Good

• great variable names, but I quibble over a couple later on
• nice use of blank lines to organize code
• appropriate use of sed. (Regex search and replace is the only time to use it.) I wouldn't expect any XML tools to be an improvement. :)
• you're using a subset of bash strict mode
• using env for portability

Questions

Some things I wish the code had comments for:

• Why do you need to set the LANG* variables?
• Why are you subtracting the $domain_linecount from the line count you got for that file? (More on this in a later bullet.)
• Explaining what the code is for in one line near the top. You included this in your code review, which is also required, but having it in the code is nice.

Suggestions

• Your du -h | gawk is doing the same thing for just different filenames. Put it in a function.
• I see from the table heading in your HTML that the line counts are unique values instead of just being a line count of that file. Instead of documenting as I encouraged above, it might be better to rename these variables ipv4_uniquelines or something that conveys the uniqueness so the subtraction doesn't look so odd.
• try shellcheck
• The "useless use of cat" report is because people would really like you to write things like wc -l filename, but that changes the output of wc and some of us like to see things flow from left to right easier, so I really wouldn't change it.
• don't call it readme.sh because then github thinks it is a readme. https://github.com/T145/the-blacklist/tree/master/scripts update_readme.sh would be fabulous.

Answer 2

Your code seems pretty much fine, and chicks has pointed out most of the issues I have. It's not a bad implementation of the approach you've chosen, but I have to wonder if that's the right approach.

HTML is a complex format, and while using regex to parse it can work as long as you've structured it in a consistent manner known in advance (which definitely seems to be the case here), couldn't it be easier to start from a template file, with easy-to-find indicators showing where to put data, and generating a new readme file by just replacing those indicators? That way, so long as the markers stay consistent it doesn't matter if the rest of the presentation is restructured, or even if you switch to an entirely different markup language, the script should continue working regardless, and you won't have to keep the script up to date as you change the text and markup around the data. It might be worth considering an approach where the last command looks more like

sed \
    -e "s/{{domain_entries}}/$domain_entries/g" \
    -e "s/{{domain_size}}/$domain_size/g" \
    -e "s/{{ipv4_entries}}/$ipv4_entries/g" \
    -e "s/{{ipv4_size}}/$ipv4_size/g" \
    -e "s/{{ipv6_entries}}/$ipv6_entries/g" \
    -e "s/{{ipv6_size}}/$ipv6_size/g" \
    scripts/README-template.md \
    > .github/README.md

Answer 3

One thing you might choose to do, given the neat correspondence of variable names and element ids, is to generate the sed script from the list of variables:

# Produce a sed script that replaces TD elements' contents with value of
# same-named variable.
# Arguments: list of variable names
make_subst_script() {
    for i
    do
        printf 's/\(<td id="%s">\).*\(<\/td>\)/\\1%s\\2/\n' "${i//_/-}" "${!i}"
    done
}

The ${i//_/-} expansion is needed, to change the _ in variable names to the - used in the HTML id attributes.

(I also used back-references to shorten the substitution, and removed the /g modifier - put that back in if you really do have lines where you need to substitute more than once).

You can then invoke sed on the result:

sed -e "$(make_subst_script domain_entries domain_size ipv4_entries ipv4_size ipv6_entries ipv6_size)" \
    -i .github/README.md

It's easy to combine this technique with template markers as recommended by Sara J.

---

We can make it slightly more efficient by eliminating useless cat:

domain_linecount=$(wc -l <black_domain.txt)

And cut is simpler and lower-overhead than the awk interpreter:

ipv4_size=$(du -h black_ipv4.txt | cut -f1)

Alternatively, instead of du on a single file, we could use stat, but then we need to transform its output to give K/M/G/etc (but with a lot more flexibility):

ipv4_size=$(stat -c %s black_ipv4.txt | numfmt --to=iec)

As chicks suggests, we should make a function for this.