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. Maybe, you should update that to work properly %q then... 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". > Anyway, correctly applying %q escaping probably isn't super-important. > But, we don't really expect repo-add or repo-remove to fail; if > something is wrong, any of the numerous checks leading up to actually > calling them should have already caught that. If we trigger one of > these error messages, something *weird* is going on, and I'd like the > most precise error message possible. No, I agree we might as well be careful here! -- Eli Schwartz Bug Wrangler and Trusted User
signature.asc
Description: OpenPGP digital signature