Re: PATCH 3/5: _imagemagick: complete all files if image files didn't match

[Mikael Magnusson <mikachu@gmail.com>]

On Sat, Aug 15, 2015 at 2:05 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Aug 15, 12:31am, Mikael Magnusson wrote:
> } Subject: Re: PATCH 3/5: _imagemagick: complete all files if image files di
> }
> } > -  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)"
> } > +  _files "$@" -g "*.(#i)(${~formats//:/|})(-.)" || _files "$@"
> }
> } Now that I'm looking at this line for the 500th time, I realize that
> } the ~ in there makes absolutely no sense, and shouldn't really work.
>
> The ~ there is in fact useless, because the expansion is quoted, so
> converting characters within the elements of $format into patterns
> doesn't do anything.
>
> In fact it's more likely that ${(b)formats} is wanted so that pattern
> charactes in each of the format strings would be quoted, if there were
> any.  (That *will* persist because it's a textual change inserting
> backslashes.)  Then they'd remain non-special when the positional
> paramter is expanded later by _files.
>
> } However, it seems to be totally ignored and harmless. I tried changing
> } formats to an array, and using this instead,
> } _files "$@" -g "*.(#i)(${(j:|:)~formats})(-.)"
>
> Same thing.
>
> } And it works the same, next I tried (actually I did the other way
> } around but this is easier to follow)
> } _files "$@" -g "*.(#i)(${(~j:|:)formats})(-.)"
>
> When you have an array, "${(j:|:)array}" generates the set consisting of
> each of the quoted array elements, and then joins them.  The vertical
> bar is not part of the quoted regions, but it's also not interpreted as
> a pattern character.
>
> "${(~j:|:)array}" generates the quoted elements and then joins them
> with the pattern character represented by vertical bar, so you get the
> expansion immediately as you noted.

Okay, I guess that makes sense on some level. :)

> } -formats=jpg:jpeg:jp2:j2k:jpc:jpx:jpf:tiff:miff:ras:bmp:cgm:dcx:ps:eps:fig:fits:fpx:gif:mpeg:pbm:pgm:ppm:pcd:pcl:pdf:pcx:png:rad:rgb:rgba:rle:sgi:html:shtml:tga:ttf:uil:xcf:xwd:xbm:xpm:yuv
> } +formats=(jpg jpeg jp2 j2k jpc jpx jpf tiff miff ras bmp cgm dcx ps
> } eps fig fits fpx gif mpeg pbm pgm ppm pcd pcl pdf pcx png rad rgb rgba
> } rle sgi html shtml tga ttf uil xcf xwd xbm xpm yuv)
>
> If you're going to do that, you should also declare formats as an array
> instead of a string, just for completeness.

Good point, I'll squash it in when pushing.

-- 
Mikael Magnusson