Re: [gentoo-dev] Re: evar_push/pop helpers

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: Text/Plain, Size: 3493 bytes --]

On Sunday 02 June 2013 13:38:04 Steven J. Long wrote:
> On Sat, Jun 01, 2013 at 11:03:20PM -0400, Mike Frysinger wrote:
> > --- eutils.eclass	22 May 2013 05:10:29 -0000	1.421
> > +++ eutils.eclass	2 Jun 2013 03:00:46 -0000
> > @@ -146,6 +146,77 @@ estack_pop() {
> >  	eval unset ${__estack_name}\[${__estack_i}\]
> >  }
> 
> Just in passing, one of the places you don't want nullglob messing things
> up.

not sure what you mean.  that escapes the [] so that bash doesn't accidentally 
glob things.  when the code actually executes, you don't need to escape 
things.

	touch f i d x
	unset arr
	declare -A arr
	arr[f]=1234
	arr[idx]=4321
	echo "${arr[f]}" "${arr[idx]}"
	__estack_name=arr
	__estack_i=f
	eval unset ${__estack_name}\[${__estack_i}\]
	echo ${#arr[@]}
	__estack_i=idx
	eval unset ${__estack_name}\[${__estack_i}\]
	echo ${#arr[@]}

seems to work

> > +# is not specified, the var will be unset.
> > +evar_push_set() {
> > +	local var=$1
> > +	evar_push ${var}
> > +	case $# in
> > +	1) unset ${var} ;;
> > +	2) eval ${var}=\$2 ;;
> 
> I wish you wouldn't use eval for this. I know it's technically okay here,
> or would be if you verified the parameter, but bash has printf -v for this
> purpose:

interesting, i hadn't seen that before ... looks new to bash-3.1.  /me tucks 
that into his tool belt.

although it doesn't quite work in the edge case where the value is an empty 
string.  consider:
	unset x
	printf -v x ''
	echo ${x+set}

that should show "set", but it does not.  i'll have to keep `eval ${var}=` 
when the value we're setting is empty.  or just keep the eval code since i 
have to do eval anyways at that point.

i'll report it upstream to the bash guys.

> printf -v "$1" '%s' "$2" 2>/dev/null || die "unable to set: '$1' to: '$2'"
> 
> Note you should verify the variable name, ime, irrespective of a die on the
> printf (or eval in sh.) It's much better feedback to the developer using
> the routine.

i don't think it's worth it.  if you screw up the name, it tends to be a one-
time cost, and the error shown is pretty clear as to where it's coming from.

> printf -v also works with array members btw, if you do decide to extend in
> that direction:

i don't think i will, but i probably can convert the other core stack code to 
use this ...

> > +		: ${cnt:=bad}
> > +		[[ -n ${cnt//[0-9]} ]] && die "${FUNCNAME}: first arg must be a
> > number: $*"
> > +		;;
> 
> Though a generic is_int function comes in much handier, ime.

yeah, and we have other code that wants this (a simple grep for 0-9 in eclass/ 
shows a bunch of hits)

> > -		# Some people like to make dirs of patches w/out suffixes (vim)
> > +		# We have to force sorting to C so that the wildcard expansion
> > 			# is consistent #471666.
> > +		evar_push_set LC_COLLATE C
> > +		# Some people like to make dirs of patches w/out suffixes (vim).
> >  		set -- "$1"/*${EPATCH_SUFFIX:+."${EPATCH_SUFFIX}"}
> > +		evar_pop
> 
> Have to say I'd just do this in a function adding to a locally-scoped
> array, if it were me, eg local args; foo "$1" "$EPATCH_SUFFIX"; set --
> "${args[@]}"; unset args
> foo() {
> 	local LC_COLLATE=C
> 	args=("$1"/*${2:+."$2"})
> }

since i plan on using these funcs in other places, using the existing api 
keeps things simple

> though I'd prefer it if EPATCH_SUFFIX were allowed to be a list,
> personally.

wouldn't really make this any easier
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]