-----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-----