Re: Unexpected Results/Heisenbug

["Bart Schaefer" <schaefer@xxxxxxxxxxxxxxxxxxxxxxx>]

On May 11,  2:48am, Ken Smith wrote:
: Subject: Unexpected Results/Heisenbug
:
: Attached is a script which surprised me when I tested it.  When run with
: DEBUG=0, the output is as I would expect.  When run with DEBUG=1, the
: for loop sets ${each} to "continue:" after the debugging code executes. 
: Is this a problem with my nubile scripting skills or does this signify a
: problem with zsh?

It's a problem with your scripting.

(Whether your skills are "of marriageable condition or age" I'll leave to
others to decide.)

Here's the bug that's confusing you:

: function format_output
: {
[...]
: 	for each in $*; do
[...]
: 	done
: }
[...]
: 
: for each in test1 test2 test3; do
[...]
: done

You've used the same parameter name in both loops.  Since you haven't
declared the one in format_output to be "local", they both refer to the
same global parameter.  ("Variables" in zsh are called "parameters" for
obscure reasons.  The terms are almost always synonymous.)

Other comments:

: function debugging
: {
: 	[[ ${+DEBUG} == 1 && ${DEBUG} == 1 ]]
: }

If the only values are integers, you can write this more succinctly using
the arithmetic syntax:

	(( DEBUG == 1 ))

: function format_output
: {
: 	function arg_len
: 	{
: 		echo $1 | wc -c
: 	}

All functions are of global scope, so declaring this one inside another
only serves to (a) delay its definition until format_output is first run
and (b) needlessly redefine it every time format_output runs again. 

Aside from that, it's completely unnecessary in zsh, because you can get
the length of any scalar (string or integer) parameter by writing $#xxx
or ${#xxx} where xxx is of course the parameter name.  So here:

: 	heading_separator=": "
: 	prefix=${1}${heading_separator}
: 	prefix_len=`arg_len ${prefix}`

	prefix_len=$#prefix

Usually, it's not even worth storing the length in another parameter.

: 	if [[ ${+COLUMNS} == 1 ]]; then
: 	columns=${COLUMNS}
: 	else
: 		columns=80
: 	fi

Fine, but you could avoid the entire if/then/else/fi by writing:

	columns=${COLUMNS:-80}

: 	terminal_position=0
: 	first_argument=1
: 	just_wrote_prefix=0

You should have declared all these parameters (plus "each") as "local",
for example:

	local heading_separator=": "

You could even go so far as to declare some of them to be integers:

    integer terminal_position=0 first_argument=1 just_wrote_prefix=0

Any such declaration within a function scope creates a local parameter,
even if it doesn't use the word "local".  But you have to preceed the
list of parameter names with one of the command words typeset, local,
integer, declare, readonly, or (in 3.1.6+) float; if you simply assign
to the name without such a declaration, you create (or re-use) a global.

: 	for each in $*; do
: 		if [[ ${first_argument} == 1 ]]; then
: 			first_argument=0
: 			continue
: 		fi

You did this to avoid writing out the first argument because it's been
stored in $prefix already.  But you could instead have discarded the
first argument before beginning the loop, by writing:

	shift

After which you don't need $first_argument, and you don't need the test
every time around the loop.

: 		if [[ ${terminal_position} == 0 ]]; then
: 			echo -n ${prefix}
: 			just_wrote_prefix=1
: 		fi
: 		token_len=`arg_len ${each}`

Once again:
		token_len=$#each

: 		((terminal_position += token_len + 1))
: 		if [[ ${terminal_position} > ${columns} ]]; then

Inside [[ ]], the > operator does string comparison, e.g. [[ 2 > 10 ]] is
true.  You probably meant:

		if (( terminal_position > columns )); then

: 			# If we just wrote the prefix, then we
: 			# have to write this token where it stands
: 			# even though it will wrap.
: 			if [[ ${just_wrote_prefix} == 1 ]]; then
: 				echo ${each}
: 				just_wrote_prefix=0
: 				((terminal_position = 0))
: 			else
: 				echo -n "\n${prefix}${each} "
: 				((terminal_position =
: 					prefix_len + token_len))
: 				just_wrote_prefix=0
: 			fi
: 		else
: 			echo -n "${each} "
: 			just_wrote_prefix=0
: 		fi
: 	done
: 	echo

All fine, except that you could have factored out the assignment to
just_wrote_prefix, since you're doing it in every possible branch.

: }
: 
: if debugging; then
: 	format_output DEBUG Executing \"${0} ${*}\"
: fi
: 
: for each in test1 test2 test3; do
: 	echo "***each=${each}"
: 	if debugging; then
: 		format_output DEBUG press enter to continue:
: 		read
: 	fi
: 	echo "***each=${each}"
: done

Also fine, provided you used "local each" in the format_output function.

Hope that helps.

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com