3
\$\begingroup\$

I have a handful of files containing simple configuration in which I need to modify values. There are some useful constraints that simplify my implementation:

  • The file consists of single lines of the form key=value (no multi-line values).
  • Key names are written using letters, digits and underscore only (no regex meta-characters).
  • Keys are always at start of line, although the = may have spaces on one or both sides.
  • All meaningful keys are already present in the file; we never need to add any lines.
  • The values I'm writing never contain backslash.

This is the function I wrote to make these changes:

# change_values FILE [KEY VALUE]...
#   Edit FILE, replacing each KEY
#   with corresponding VALUE.
change_values()
{
    file=${1?change_values requires a file name}; shift
    sed -f <(printf '/^%s *=/s/=.*/= %s/\n' "${@//\//\\/}") \
        -i "$file"
}

To demonstrate its operation, I used this simple test:

set -eu

d=$(mktemp -d)
trap 'rm -r "$d"' EXIT
cd "$d"

cat >config <<END
# This is a comment
tmpdir=/var/tmp
command = stat
alt_command = rm
# Spaces around = are ignored
format= %c%d
format2 =%s%d
END

change_values config \
              tmpdir '/tmp/workdir' \
              format "%d%s" \
              command 'ls' \
              nonexistent 'no_effect'

cmp config - <<END
# This is a comment
tmpdir= /tmp/workdir
command = ls
alt_command = rm
# Spaces around = are ignored
format= %d%s
format2 =%s%d
END
\$\endgroup\$
7
  • \$\begingroup\$ I cannot really review bash, its always been a cryptic mess to me... However I have one note. These "key=value" config files often contain commented out lines, at least those I've seen. Do you expect commented out lines? Would you prefer to ignore such lines or replace anyway? In either case I suggest adding a test for it. \$\endgroup\$
    – slepic
    Jun 6, 2023 at 14:21
  • \$\begingroup\$ In this case, we might have comment lines, but not "commented-out" lines (all the meaningful keys are present as keys). I should probably add a test that demonstrates the comment lines don't get modified. \$\endgroup\$ Jun 6, 2023 at 14:54
  • \$\begingroup\$ I've made the test self-checking now. \$\endgroup\$ Jun 6, 2023 at 14:59
  • \$\begingroup\$ The sed s command is crystal clear and the test is lovely, thank you, looks good, especially the "no comments!" ^ caret anchor. shifting the filename out of $@ made perfect sense. I imagine that ${@//\//\\/} does the Right Thing, but as expressed it is obscure, and if double backwhack is going to be so troublesome perhaps this isn't the appropriate tool for the job. I would neither delegate nor accept maintenance tasks on this code. \$\endgroup\$
    – J_H
    Jun 6, 2023 at 21:07
  • \$\begingroup\$ Did you consider TOML tools like crates.io/crates/toml-cli ? Your key value pairs look very close to TOML files. \$\endgroup\$
    – chicks
    Jun 7, 2023 at 15:05

1 Answer 1

2
\$\begingroup\$

It's a nice idea, simple key=value pairs are a common configuration format.

Declare local variables

I recommend to always declare local variables inside functions to avoid accidentally overwriting values in the global namespace.

Adding more validation

The function only checks that it's called with at least one parameter.

Admittedly these are minor issues:

  • The file will be overwritten even when there are no key-value specified
  • The value will be empty when the value is missing (odd number of remaining parameters after shift)
  • The error message is not helpful when the first parameter is the empty string

I would add a bit more validation mostly to get a signal in case of accidental misuses. For example if I call this from a script, and due to a bug it's not passing sensible arguments, then the validation would signal that something's off, rather than quietly do nothing.

Adopting a consistent config formatting convention

In a config file of key-value pairs I would expect one of the following formatting styles:

  • key=value
  • key = value

The script transforms key=value to key= value which I find a bit surprising, and an odd hybrid style.

I would either preserve the existing style, or apply one of the common styles (easier).

Beware of GNU vs BSD

The usage pattern of the -i flag of sed in this script requires the GNU implementation. I would either mention this somewhere, or adjust the script to make it GNU/BSD agnostic.

Improving the test script

I know the test script is just something you quickly threw together, I would still improve it as a matter of principle, because it's easy enough to do.

cd is not recommended in scripts, and it's easy enough to avoid by changing references of config to "$d/config". This would have the added benefit of making it more clear that it's a file path, not some keyword.

The names could be improved:

  • d -> tmp
  • config -> example.conf

In the here-documents I would use << "EOF" to protect myself in case I might accidentally include $ anywhere in the example content.

I generally recommend activating the -o pipefail option.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for the review. I'd almost forgotten that I'd asked this one! cd isn't a problem in scripts as long as failure is handled (here by set -e) - but I do agree that using a full path makes the test program clearer; I'll make that change. \$\endgroup\$ Sep 4, 2023 at 7:03

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.