Re: [gentoo-dev] enhancement for doicon/newicon in eutils.eclass

[hasufell]

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