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

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to