Re: [BUG] getopts OPTIND

[dana <dana@dana.is>]

(Resurrecting this per workers/48509)

bin_getopts() has changed a little since i posted my patch before, this looks
a bit weirder. The test is also weird. But i confirmed that this makes it
behave like dash, bash, and mksh (my ksh93 doesn't support local, and i still
don't know what yash is doing):

% pbpaste | zsh-dev -f
<1><1><3><5><7><6>
% pbpaste | zsh-dev -f --posix-builtins
<2><2><3><6><7><7>
% pbpaste | dash
<2><2><3><6><7><7>
% pbpaste | bash
<2><2><3><6><7><7>
% pbpaste | mksh
<2><2><3><6><7><7>
% pbpaste | yash
<3><3><3><7><7><7>

tbh i don't think i fully understand why this needs to work this way, or
whether there are other cases that should be tested. Open to review obv

dana


diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index a7afe42cf..4b9778e40 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -982,7 +982,8 @@ vindex(OPTARG, use of)
 The first option to be examined may be changed by explicitly assigning
 to tt(OPTIND).  tt(OPTIND) has an initial value of tt(1), and is
 normally set to tt(1) upon entry to a shell function and restored
-upon exit (this is disabled by the tt(POSIX_BUILTINS) option).  tt(OPTARG)
+upon exit.  (The tt(POSIX_BUILTINS) option disables this, and also changes
+the way the value is calculated to match other shells).  tt(OPTARG)
 is not reset and retains its value from the most recent call to
 tt(getopts).  If either of tt(OPTIND) or tt(OPTARG) is explicitly
 unset, it remains unset, and the index or option argument is not
diff --git a/Src/builtin.c b/Src/builtin.c
index 26335a2e8..13dfdf8be 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -5556,6 +5556,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     /* check for legality */
     if(opch == ':' || !(p = memchr(optstr, opch, lenoptstr))) {
 	p = "?";
+	/* Keep OPTIND correct if the user doesn't return after the error */
+	if (isset(POSIXBUILTINS)) {
+	    optcind = 0;
+	    zoptind++;
+	}
 	zsfree(zoptarg);
 	setsparam(var, ztrdup(p));
 	if(quiet) {
@@ -5572,6 +5577,11 @@ bin_getopts(UNUSED(char *name), char **argv, UNUSED(Options ops), UNUSED(int fun
     if(p[1] == ':') {
 	if(optcind == lenstr) {
 	    if(!args[zoptind]) {
+		/* Fix OPTIND as above */
+		if (isset(POSIXBUILTINS)) {
+		    optcind = 0;
+		    zoptind++;
+		}
 		zsfree(zoptarg);
 		if(quiet) {
 		    setsparam(var, ztrdup(":"));
diff --git a/Test/B10getopts.ztst b/Test/B10getopts.ztst
index 72c9e209e..69b3d63f4 100644
--- a/Test/B10getopts.ztst
+++ b/Test/B10getopts.ztst
@@ -96,3 +96,29 @@
   done
 0:missing option-argument (quiet mode)
 >:,x
+
+  # This function is written so it can be easily referenced against other shells
+  t() {
+    local o i=0 n=$1
+    shift
+    while [ $i -lt $n ]; do
+      i=$(( i + 1 ))
+      getopts a: o "$@" 2> /dev/null
+    done
+    printf '<%d>' "$OPTIND"
+  }
+  # Try all these the native way, then the POSIX_BUILTINS way
+  for 1 in no_posix_builtins posix_builtins; do (
+    setopt $1
+    print -rn - "$1: "
+    t 1 -a
+    t 1 -w
+    t 2 -a -w
+    t 4 -a -w -e -r -a
+    t 5 -a -w -e -a -w -e
+    t 5 -a -w -e -r -ax -a
+    print
+  ); done
+0:OPTIND calculation with and without POSIX_BUILTINS (workers/42248)
+>no_posix_builtins: <1><1><3><5><7><6>
+>posix_builtins: <2><2><3><6><7><7>