|
| 1 | +# Feedback from dotfiles utilities |
| 2 | + |
| 3 | +Issues and improvement candidates discovered while building bash utilities |
| 4 | +in the [dotfiles](https://github.com/uhop/dotfiles) project. |
| 5 | + |
| 6 | +--- |
| 7 | + |
| 8 | +## `ansi::err` returns 1 — footgun with `set -e` |
| 9 | + |
| 10 | +`ansi::err()` in `ansi-utils.sh` always returns 1. All dotfiles utilities |
| 11 | +use `set -euCo pipefail`, so any `ansi::err` call that isn't the last |
| 12 | +statement before `exit` kills the script unexpectedly. |
| 13 | + |
| 14 | +**Symptom:** Multi-line error messages (several `ansi::err` calls before |
| 15 | +`exit 1`) only print the first line — the second call triggers `set -e`. |
| 16 | + |
| 17 | +**Workaround:** `ansi::err "message" || true` or group with |
| 18 | +`{ ...; } || true`. |
| 19 | + |
| 20 | +**Possible fixes:** |
| 21 | +- Add `ansi::warn` (or similar) that prints to stderr but returns 0. |
| 22 | + Reserve `ansi::err` for the "print and signal failure" use case. |
| 23 | +- Add an option/flag to suppress the return code. |
| 24 | +- Change `ansi::err` to return 0 by default (breaking change — would need |
| 25 | + to audit callers that rely on the return 1 behavior). |
| 26 | + |
| 27 | +**Note:** `ansi::err` accepts multiline strings, which reduces multi-call |
| 28 | +sequences to one call. But the `|| true` is still needed if there's code |
| 29 | +after the call. |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +## `args::parse` corrupts positional arguments containing spaces |
| 34 | + |
| 35 | +In `args.sh`, `args::parse` escapes non-flag positional arguments with |
| 36 | +`printf '%q'` before passing to `getopt` (lines 160-167). The output is |
| 37 | +then processed with `eval set -- "$parsed"` (line 183). The round-trip |
| 38 | +doesn't work for strings with spaces: |
| 39 | + |
| 40 | +1. `printf '%q' "echo hello"` → `echo\ hello` |
| 41 | +2. `getopt` wraps it in single quotes: `'echo\ hello'` |
| 42 | +3. `eval set --` processes the single quotes, but single quotes don't |
| 43 | + interpret backslash escapes, so the result is literally `echo\ hello` |
| 44 | + |
| 45 | +**Minimal repro:** |
| 46 | + |
| 47 | +```bash |
| 48 | +source bootstrap.sh |
| 49 | +args::program "test" "1.0" "test" |
| 50 | +args::option "-h, --help" "help" |
| 51 | +args::parse "echo hello from periodic" |
| 52 | +echo "${args_cleaned[0]}" |
| 53 | +# Expected: echo hello from periodic |
| 54 | +# Actual: echo\ hello\ from\ periodic |
| 55 | +``` |
| 56 | + |
| 57 | +**Workaround:** `"${2//\\ / }"` to strip `\ ` → ` ` in the caller. |
| 58 | + |
| 59 | +**Possible fix:** In `args::parse`, don't `printf '%q'` the positional |
| 60 | +arguments before `getopt`. The escaping was likely added to protect |
| 61 | +arguments from `eval set --`, but `getopt` already single-quotes them in |
| 62 | +its output, so the extra escaping creates a double-quoting problem. |
0 commit comments