Re: [gentoo-dev] Re: RFD: split out some functions from eutils.eclass?

[Michał Górny]

On Wed, 11 Jan 2012 23:09:46 +0100
Ulrich Mueller <ulm@g.o> wrote:

> # @ECLASS: cdrom.eclass
> # @MAINTAINER:
> # games@g.o
> # @BLURB: Functions for cdrom handling
> # @DESCRIPTION:
> # Acquire cd(s) for those lovely cd-based emerges.  Yes, this violates
> # the whole 'non-interactive' policy, but damnit I want CD support!

Maybe the eclass should now set PROPERTIES=interactive?

> cdrom_get_cds() {
> 	# first we figure out how many cds we're dealing with by
> 	# the # of files they gave us
> 	local cdcnt=0
> 	local f=
> 	for f in "$@" ; do
> 		((++cdcnt))
> 		export CDROM_CHECK_${cdcnt}="$f"
> 	done
> 	export CDROM_TOTAL_CDS=${cdcnt}
> 	export CDROM_CURRENT_CD=1

Why are you exporting all that? I don't think that should be necessary
unless you subshell somewhere.

CDROM_CHECK=( "${@}" )
CDROM_TOTAL_CDS=${#} # (or ${#CDROM_CHECK[@]})

And then you can just refer to ${CDROM_CHECK[$i]} rather than using all
those ugly ${!...}.

> 	# now we see if the user gave use CD_ROOT ...
> 	# if they did, let's just believe them that it's correct
> 	if [[ -n ${CD_ROOT}${CD_ROOT_1} ]] ; then
> 		local var=
> 		cdcnt=0
> 		while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do
> 			((++cdcnt))
> 			var="CD_ROOT_${cdcnt}"
> 			[[ -z ${!var} ]] && var="CD_ROOT"
> 			if [[ -z ${!var} ]] ; then
> 				eerror "You must either use just the
> CD_ROOT" eerror "or specify ALL the CD_ROOT_X variables."
> 				eerror "In this case, you will need" \
> 					"${CDROM_TOTAL_CDS} CD_ROOT_X
> variables." die "could not locate CD_ROOT_${cdcnt}"
> 			fi
> 		done
> 		export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}}
> 		einfo "Found CD #${CDROM_CURRENT_CD} root at
> ${CDROM_ROOT}"

#1. You are using 1 in variable calls, I don't see a reason to use
longer ${CDROM_CURRENT_CD} in this one output case.

>		export CDROM_SET=-1
> 		for f in ${CDROM_CHECK_1//:/ } ; do
> 			((++CDROM_SET))

More when I get replies on these ones.

-- 
Best regards,
Michał Górny