Comment by hddqsb | Hacker News Reader

Comment by hddqsb | Hacker News Reader

Parent Full thread

Note that `test -v` is not in POSIX sh (see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/t...). And you don't need `builtin` because `test ...` invokes the builtin by default; if you want to run coreutils test you need to use `/usr/bin/test ...` or `env test ...` or `(exec test ...)`.

The usual way to check if $2 is present is to use `[ $# -ge 2 ]`. For named variables, I like to use `[ "${myvar+set}" != "set" ]` (the parameter expansion `${myvar+word}` expands to "word" if myvar is set or "" if it is unset, see https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V...).

(Often people want to treat an empty value as unset, so they just use `[ -n "$myvar" ]` to check that the value is non-empty. If you enable `set -u` to treat references to unset variables as errors, this should instead be `[ -n "${myvar-}" ]`.)

Some comments on the script you linked:

I highly recommend ShellCheck (https://www.shellcheck.net/), it picks up a legitimate issue in your script: `tr -s [:alpha:] Y` should be `tr -s "[:alpha:]" Y` (otherwise it fails if the current directory contains a single-letter filename).

Also, you should use `printf "%s\n" "$var"` instead of `echo "$var"` in case $var is "-n" for example.

All great comments, thank you! I feel the need to reply, even though there isn't a real need to, but here goes:

1. True; but in this case we're talking about bash specifically, so I prefer to use cleaner syntax whenever this is available. I find ${var+x} a bit too hacky ...

2. Huh, I was under the impression that bespoke commands overrode builtins. Good to know. What the hell is even the reason to have a 'builtin' keyword then ... just for "alias test=/usr/bin/test" style scenarios??

3. True about $#. Not sure why I didn't think of that, d'oh.

4. Thanks re Shellcheck. I do have it in mind, and was planning to use it prior to releasing a v1; but process_optargs came out of a larger project which is still in v0 (https://git.sr.ht/~tpapastylianou/bashtickets), and I only forked it into its own thing because someone asked me about it on HN, so I lost track. Thanks for the reminder. Having said that, I can't see why the error you describe would occur in this instance. Would you mind explaining? Why would the presence of a single-letter filename have anything to do with the piped output to tr?

5. Good point about printf (or about habitually passing "--" as an argument to things, I guess).

Thanks again!

> I can't see why the error you describe would occur in this instance. Would you mind explaining?

Sure, it's due to pathname expansion. The most commonly used pattern is `*` (e.g. `echo *.txt` might expand to `echo foo.txt bar.txt`), but there is also `[...]` for matching a character range. If the current directory contains files named "i" and "j", then `tr -s [:alpha:] Y` will expand to `tr -s i j Y`, which is not what you want. (The reason it works when the current directory doesn't contain single-letter filenames is that a pattern which doesn't match any filenames is left as-is.)

Ah right, gotcha. Actually I think `i` and `j` would be fine, since the whole point is that [:alpha:] will expand to any of the characters contained in that range before it has a chance to be interpreted as the intended character class (and therefore, i and j are safe from accidental pathname expansion, since they don't appear in that range).

But yes, this indeed causes problems if you have a file called `:`, `a`, `l`, `p`, or `h` on the system.

Good catch. Thanks again.

Oops, quite right -- the glob pattern for matching any letter is `[[:alpha:]]`, not `[:alpha:]`. Cheers!

Edit: That should have been `[ "${myvar+set}" = "set" ]` for checking that `myvar` is set.

Reply on news.ycombinator.com