-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05/22/2012 04:49 AM, Mike Frysinger wrote:
> On Sunday 20 May 2012 19:24:13 hasufell wrote:
>> case ${2} in
> 
> please use $1/$2/etc... with positional variables when possible
> 
>> 16|22|24|32|36|48|64|72|96|128|192|256) size=${2}x${2};; 
>> 16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|128x128|
> 192x192|256x256)
>> size=${2};; scalable) size=scalable;; *) eqawarn "${2} is an
>> unsupported icon size!" ((++ret));; esac
> 
> you can write this w/out having to duplicate two lists: size= if [[
> $2 == "scalable" ]] ; then size=$2 elif [[ ${2:0:2}x${2:0:2} ==
> "$2" ]] ; then size=${2:0:2} case ${size} in 
> 16|22|24|32|36|48|64|72|96|128|192|256) ;; *) size= ;; esac fi if
> [[ -z ${size} ]] ; then eqawarn "${2} is an unsupported icon
> size!" ((++ret)) fi shift 2
> 
> shift 2;; -t|--theme) theme=${2} shift 2;; -c|--context) 
> context=${2} shift 2;; *)
>> if [[ -z ${size} ]] ; then dir=/usr/share/pixmaps else 
>> dir=/usr/share/icons/${theme}/${size}/${context} fi  insinto
>> "${dir}"
> 
> considering you only use $dir once, you could just call `insinto`
> directly on the path rather than using the dir variable at all
> 
>> elif [[ -d ${1} ]] ; then for i in "${1}"/*.{png,svg} ; do doins
>> "${i}" ((ret+=$?)) done
> 
> why loop ?  `doins "${1}"/*.{png,svg}` works just as well
> 
> you probably want to enable nullglobbing here, otherwise this will
> cause problems if you try to doicon on a dir that contains just
> svg.
> 
> also, what about other file types ?  people install xpm, svgz, gif,
> and other file types ...
> 
>> exit ${ret}
> 
> bash masks error codes to [0..255], so all the ret updates should
> probably be changed to just: ret=1
> 
> after all, i doubt anyone cares how many errors there were, just
> that one occurred.  and while you're here, might want to make it
> auto die on failure like we've done with all our other helpers. 
> -mike

Thanks, I'v implemented most of that, but your proposal about
non-duplicated list in case) has multiple problems. The only cases
that actually work with that snippet are:
16x16|22x22|24x24|32x32|36x36|48x48|64x64|72x72|96x96|scalable.
All others will fail (like 128x128 or just 48).
So it would end up like this:

case $1 in
                -s|--size)
                        if [[ ${2:0:2}x${2:0:2} == "$2" ]] ; then
                                size=${2:0:2}
                        elif [[ ${2:0:3}x${2:0:3} == "$2" ]] ; then
                                size=${2:0:3}
                        else
                                size=${2}
                        fi
                        case ${size} in
                        16|22|24|32|36|48|64|72|96|128|192|256)
                                size=${size}x${size};;
                        scalable)
                                ;;
                        *)
                                eerror "${size} is an unsupported icon size!"
                                exit 1;;
                        esac
                        shift 2;;
                -t|--theme)


This does not really look cleaner than just using two lists. I would
prefer the latter for readability.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJPvX3MAAoJEFpvPKfnPDWzxvMH/16kN1Zkby6LHg2Ev7H2qNPh
ajbqVonTuuLnIVxEwXYXYABEkF+qwD5xnJPMEclvkn8FXAVerFeyaxJgBelldXnr
DJMHiPhz0umJaMfvAFrEsbIo5IrxKMTpMMj3fuu5ruQMrSboV4alPSM7l2haXZ5W
3TbfbFmWoQzft1DolDlFb38M0TtRko7viZ1KQJUZjxCEClh8tEiOrQVxR8xcoi33
MiwEVZlib4KnWetq3qGZdU+xRFi/yzUmtFVv0pfbYIV51w4KHoi8cD6OkpiVzLdI
bhWCmyDeKq6wOcfXfcfGKzYc+2M/hP8xkhiG3/KjDXe6FUzdG63+U1Wmu521VDM=
=Rn8t
-----END PGP SIGNATURE-----

Reply via email to