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

Reply via email to