On Thu, 22 Feb 2018 19:23:17 -0500, Eli Schwartz wrote:
On 02/22/2018 06:54 PM, Luke Shumaker wrote:
I do see what you're doing, I'm just not sure why. Is the whole idea with this extra variable floating around, to avoid tokenizing "${pkgs[@]}" as separate messages? That's why "${pkgs[*]}" tokenizes the members of an array as one word by gluing the members together using the first IFS character (which is a space). You'll note I used this in testing2x.
As for using %q for filepaths that can theoretically contain spaces... good point I guess.
It's all about %q. (I did use ${ary[*]} somewhere else in the commit). The separate variable applies %q escaping to each package filename separately. Without it, if I did something like:
+ || error 'repo-remove %q %q' "$dbfile" "${pkgs[*]}"
Then it would also escape the spaces that separate them.
But, you're using
error 'repo-remove %q %s' "$dbfile" "${pkgs_str% }"
with a %s which works just as well as it ever did. And you might as well do that with "${pkgs[*]}" since that also works as well as it ever did.
I guess I *should* have explained it a bit more; the escaping of the package list happens when assigning pkgs_str: printf -v pkgs_str -- '%q ' "${pkgs[@]}"
Or maybe, you should skip the temporary variable and use
error 'repo-remove %s %s' "${dbfile@Q}" "${pkgs[*]@Q}"
This will result in the variables being passed into error, after being suitably '/path quoted/rather/than/escaped/'
See the bash documentation on "Parameter transformation".
Oohh, a new Bash 4.4 feature that I missed. (It's been >1 year, I don't have an excuse). And, to be fair, I had written those ~4 lines of code (f338cb0 in Parabola's dbscripts) before Bash 4.4 existed! (I knew that I had written code to escape the package list before, and just grabbed those lines from our dbscripts.) @Q generally escapes things by wrapping them in single-quotes; %q generally escapes them by inserting backslashes. Anyway, your simpler version LGTM. Shall I submit a v2 patchset? -- Happy hacking, ~ Luke Shumaker