_make-expandVars() broken? (was Re: Makefile completion with wildcard targets)

["Jun T." <takimoto-j@xxxxxxxxxxxxxxxxx>]

Continued form zsh-users (20348, 20349, 20352).

Currently _make recognizes the target by the the following
pattern (line 87 of _make):

[[:alnum:]][^$TAB:=]#:[^=]*

i.e., the first character must be an alnum. This means if a target
start with a meta character, for example,

*.c: dependencies

then *.c is not recognized as a target. If we are going to add -Q
to compadd, then it would be better to use a pattern something like

[[*?[:alnum:]$][^$TAB:=%]#:[^=]*

But when I tried this pattern I noticed that the function
_make-expandVars() was not working at all.

# I've looked into the function but couldn't understand what
# the variables $front, $ret and $tmp were supposed to do.

I think this function must be fixed irrespective of what pattern
to use at line 87, and add -Q to compadd or not.

I tried to rewrite the function as in the patch below.
It seems to work at least with the zsh's top-level Makefile
and several of my own small Makefiles, but may be far from
complete. Please improve if someone has a spare time.

In _make-expandVars() below, the input is divided into $front$rest,
where in $front all the make variables have already been replaced
with their values, while in $rest there remains one or more variables. 

I also changed the $TARGETS from an associative to an ordinary
array, because only its keys were used.


diff --git a/Completion/Unix/Command/_make b/Completion/Unix/Command/_make
index c14a34c..64c91cd 100644
--- a/Completion/Unix/Command/_make
+++ b/Completion/Unix/Command/_make
@@ -4,58 +4,57 @@
 # are used in those targets and their dependencies.
 
 _make-expandVars() {
-  local open close var val front ret tmp=$1
+  local open close var val front='' rest=$1
 
-  front=${tmp%%\$*}
-  case $tmp in
-    (\(*) # Variable of the form $(foobar)
-    open='('
-    close=')'
-    ;;
-
-    ({*) # ${foobar}
-    open='{'
-    close='}'
-    ;;
-
-    ([[:alpha:]]*) # $foobar. This is exactly $(f)oobar.
-    open=''
-    close=''
-    var=${(s::)var[1]}
-    ;;
-
-    (\$*) # Escaped $.
-    print -- "${front}\$$(_make-expandVars ${tmp#\$})"
-    return
-    ;;
-
-    (*) # Nothing left to substitute.
-    print -- $tmp
-    return
-    ;;
-  esac
+  while [[ $rest = (#b)[^$]#($)* ]]; do
+    front=$front${rest[1,$mbegin[1]-1]}
+    rest=${rest[$mbegin[1],-1]}
 
-  if [[ -n $open ]]
-  then
-    var=${tmp#$open}
-    var=${var%%$close*}
-  fi
+    case $rest[2] in
+      ($)	    # '$$'. may not appear in target and variable's value
+	front=$front\$	    # or $front\$\$ ?
+	rest=${rest[3,-1]}
+	continue
+	;;
+      (\()	    # Variable of the form $(foobar)
+	open='('
+	close=')'
+	;;
+      ({)	    # ${foobar}
+	open='{'
+	close='}'
+	;;
+      ([[:alpha:]]) # $foobar. This is exactly $(f)oobar.
+	open=''
+	close=''
+	var=$rest[2]
+	;;
+      (*)	    # bad parameter name
+	print -- $front$rest
+	return
+	;;
+    esac
 
-  case $var in
-    ([[:alnum:]_]#)
-    val=${VARIABLES[$var]}
-    ret=${ret//\$$open$var$close/$val}
-    ;;
+    if [[ -n $open ]]; then
+      if [[ $rest = \$$open(#b)([[:alnum:]_]##)(#B)$close* ]]; then
+	var=$match
+      else  # unmatched () or {}, or bad parameter name
+	print -- $front$rest
+	return
+      fi
+    fi
 
-    (*)
-    # Improper variable name. No replacement.
-    # I'm not sure if this is desired behavior.
-    front+="\$$open$var$close"
-    ret=${ret/\$$open$var$close/}
-    ;;
-  esac
+    if [[ -n ${VARIABLES[(i)$var]} ]]; then
+      rest=${rest//\$$open$var$close/${VARIABLES[$var]}}
+    else
+      # if $var is not defined, just leave it as is.
+      # (or better to replace by a null string?)
+      front=$front\$$open$var$close
+      rest=${rest#\$$open$var$close}
+    fi
+  done
 
-  print -- "${front}$(_make-expandVars ${ret})"
+  print -- ${front}${rest}
 }
 
 _make-parseMakefile () {
@@ -84,16 +83,9 @@ _make-parseMakefile () {
 
       # TARGET: dependencies
       # TARGET1 TARGET2 TARGET3: dependencies
-      ([[:alnum:]][^$TAB:=]#:[^=]*)
-      input=$(_make-expandVars $input)
-      target=${input%%:*}
-      dep=${input#*:}
-      dep=${(z)dep}
-      dep="$dep"
-      for tmp in ${(z)target}
-      do
-        TARGETS[$tmp]=$dep
-      done
+      ([[*?[:alnum:]$][^$TAB:=%]#:[^=]*)
+      target=$(_make-expandVars ${input%%:*})
+      TARGETS+=( ${(z)target} )
       ;;
 
       # Include another makefile
@@ -150,7 +142,8 @@ _make() {
   local prev="$words[CURRENT-1]" file expl tmp is_gnu dir incl match
   local context state state_descr line
   local -a option_specs
-  local -A TARGETS VARIABLES opt_args
+  local -A VARIABLES opt_args
+  local -aU TARGETS
   local ret=1
 
   _pick_variant -r is_gnu gnu=GNU unix -v -f
@@ -275,7 +268,7 @@ _make() {
       while _tags
       do
         _requested targets expl 'make targets' \
-          compadd -- ${(k)TARGETS} && ret=0
+          compadd -Q -- ${TARGETS} && ret=0
         _requested variables expl 'make variables' \
           compadd -S '=' -- ${(k)VARIABLES} && ret=0
       done