1 |
On Thursday 12 January 2012 01:49:59 Michał Górny wrote: |
2 |
> On Wed, 11 Jan 2012 23:09:46 +0100 Ulrich Mueller wrote: |
3 |
> > cdrom_get_cds() { |
4 |
> > |
5 |
> > # first we figure out how many cds we're dealing with by |
6 |
> > # the # of files they gave us |
7 |
> > local cdcnt=0 |
8 |
> > local f= |
9 |
> > for f in "$@" ; do |
10 |
> > |
11 |
> > ((++cdcnt)) |
12 |
> > export CDROM_CHECK_${cdcnt}="$f" |
13 |
> > |
14 |
> > done |
15 |
> > export CDROM_TOTAL_CDS=${cdcnt} |
16 |
> > export CDROM_CURRENT_CD=1 |
17 |
> |
18 |
> Why are you exporting all that? I don't think that should be necessary |
19 |
> unless you subshell somewhere. |
20 |
|
21 |
technically, the "export" can probably be removed. it does help to clearly |
22 |
mark the variables that are "stateful" across multiple/different calls, but |
23 |
that's probably also done via the "CDROM_XXX" naming. i'm indifferent. |
24 |
|
25 |
> And then you can just refer to ${CDROM_CHECK[$i]} rather than using all |
26 |
> those ugly ${!...}. |
27 |
|
28 |
yes, i wrote this before i had a grasp of bash arrays. however, for Ulrich's |
29 |
work, he should just be relocating code without modifying. if we want to do |
30 |
cleanups, it'd be a follow up. i'm sure i could rewrite quite a bit now just |
31 |
by reading it. |
32 |
|
33 |
> > # now we see if the user gave use CD_ROOT ... |
34 |
> > # if they did, let's just believe them that it's correct |
35 |
> > if [[ -n ${CD_ROOT}${CD_ROOT_1} ]] ; then |
36 |
> > local var= |
37 |
> > cdcnt=0 |
38 |
> > while [[ ${cdcnt} -lt ${CDROM_TOTAL_CDS} ]] ; do |
39 |
> > ((++cdcnt)) |
40 |
> > var="CD_ROOT_${cdcnt}" |
41 |
> > [[ -z ${!var} ]] && var="CD_ROOT" |
42 |
> > if [[ -z ${!var} ]] ; then |
43 |
> > eerror "You must either use just the |
44 |
> > CD_ROOT" eerror "or specify ALL the CD_ROOT_X variables." |
45 |
> > eerror "In this case, you will need" \ |
46 |
> > "${CDROM_TOTAL_CDS} CD_ROOT_X |
47 |
> > variables." die "could not locate CD_ROOT_${cdcnt}" |
48 |
> > fi |
49 |
> > done |
50 |
> > export CDROM_ROOT=${CD_ROOT_1:-${CD_ROOT}} |
51 |
> > einfo "Found CD #${CDROM_CURRENT_CD} root at |
52 |
> > ${CDROM_ROOT}" |
53 |
> |
54 |
> #1. You are using 1 in variable calls, I don't see a reason to use |
55 |
> longer ${CDROM_CURRENT_CD} in this one output case. |
56 |
|
57 |
i don't know what you mean |
58 |
-mike |