Steve Long kirjoitti: > Alistair Bush wrote: >> Im sure Elvanor can't wait for you constructive feedback on his eclass >> and depending on your feedback the eclass will enter the tree this >> weekend. >> > A couple of very minor performance points, which I think are more > significant in eclasses. Firstly the basename thing Donnie pointed out > before:
Well usually you don't noticy any differences. In global scope things
could have some effect of course.
>
> You have a couple of functions that take, say, 4 or 5 arguments. It would be
> more robust to use a case, eg:
> case $# in
> 5)..;;
> 4)..;;
> *) die "Incorrect use of $FUNCNAME";;
> esac
> ..than if (($#>4)); then ..; else .. ;fi
>
Well imho they are equivalent and eclass maintainers should go with what
they prefer.
>
> With regard to:
> debug-print-function ${FUNCNAME} $*
> if you want debug-print-function to get the arguments correctly, use "$@"
> not $* (cf 'Special Parameters' in man bash for a proper explanation: this
> applies to all arrays. http://wooledge.org/mywiki/BashFAQ/073 shows how to
> manipulate all members of an array, eg to add a prefix.)
Doesn't matter as arguments are just written to a file and for some
reason this form is used all over the base in eclasses but shouldn't
hurt to use "[EMAIL PROTECTED]" with new code.
>
> This use of counter in _java-pkg_osgijar-fromfile is odd:
> while [[ -n "${1}" ]]; do
> # while [[ $1 ]] or while (($#)) also work
> if [[ "${1}" == "--noversion" ]]; then
> noversion=1
> else
> arguments[${counter}]="${1}"
> ((++counter))
> fi
> shift 1 # just shift?
> done
>
> (([EMAIL PROTECTED] < 3)) && die "At least three arguments (not
> counting --noversion) are needed for java-pkg_osgijar-fromfile()"
>
> You can either just add to the array with: arguments+=("$1")
> or add using the counter: arguments[counter++]="$1"
> and then check: ((counter < 3)) && die ..
>
> Arithmetic context[1] applies in array indexes, so you don't need to use a $
> for variables and you can post/pre-incr etc there.
> Yuu can also use it for C-style flags, with 0 as false and non-zero true:
> if [[ "${noversion}" == 1 ]];
> can be
> if ((noversion));
>
> This is handy since unset variables evaluate as zero, which is false inside
> ((..)), so a simple flag=1 is all that's needed, ie you don't have to set a
> default, although ofc it's more robust to do so, especially for functions.
> declare -i flag=0 # makes a local var which can only have integer values
> assigned (setting it to string typically evaluates to zero; arithmetic
> context is used for all assignments, so a string which happens to be a
> variable with an integer will return that value.)
>
> [1] http://wooledge.org/mywiki/ArithmeticExpression
>
We really don't need to create the arguments array at all. We could just
check if ${1} is --noversion and then use shift. After that we can use
the positional arguments as usual so instead of ${arguments[0]} we can
use just ${1}.
Regards,
Petteri
signature.asc
Description: OpenPGP digital signature
