On Wed, 11 Jan 2012 23:09:46 +0100
Ulrich Mueller <[email protected]> wrote:

> # @ECLASS: cdrom.eclass
> # @MAINTAINER:
> # [email protected]
> # @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

Attachment: signature.asc
Description: PGP signature

Reply via email to