Re: [PATCH] don't exit shell on [[ -o invalid@option ]]

[Daniel Shahaf <d.s@daniel.shahaf.name>]

Martijn Dekker wrote on Thu, 16 Nov 2017 02:52 +0200:
> Op 15-11-17 om 01:52 schreef Daniel Shahaf:
> > Okay, so how about if we demoted the fatal error to a warning?  Like
> > this:
> > 
> >     % [[ -o not_an_option ]] || echo This gets run
> >     zsh: [[: no such option: not_an_option
> >     This gets run
> >     % 
> 
> I think that would be fine, as well as returning status 2 along with it.
> 
> I do think the warning (and the status 2) should be suppressed if
> POSIX_IDENTIFIERS or some other emulation-relevant shell option is
> active, so that 'emulate ksh' and 'emulate sh' reproduce ksh behaviour.

How about the following.

It uses status 3 because status 2 currently means "syntax error in [["
and I didn't want to overload that; and it uses POSIX_BUILTINS because
that seemed more closely related than POSIX_IDENTIFIERS.

Consider it a work in progress, i.e., particulars are still malleable.
There are good arguments in favour of all sides here… (incumbent
behaviour, Martijn's patch, this patch)

Cheers,

Daniel

diff --git a/Doc/Zsh/cond.yo b/Doc/Zsh/cond.yo
index e08fc0d36..4ca132a26 100644
--- a/Doc/Zsh/cond.yo
+++ b/Doc/Zsh/cond.yo
@@ -45,6 +45,10 @@ item(tt(-o) var(option))(
 true if option named var(option) is on.  var(option)
 may be a single character, in which case it is a single letter option name.
 (See noderef(Specifying Options).)
+
+When no option named var(option) exists, and the tt(POSIX_BUILTINS) option
+hasn't been set, return 3 with a warning.  If that option is set, return 1
+with no warning.
 )
 item(tt(-p) var(file))(
 true if var(file) exists and is a FIFO special file (named pipe).
diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo
index 28155d796..d043cf398 100644
--- a/Doc/Zsh/options.yo
+++ b/Doc/Zsh/options.yo
@@ -2169,6 +2169,9 @@ command found in the path.
 Furthermore, the tt(getopts) builtin behaves in a POSIX-compatible
 fashion in that the associated variable tt(OPTIND) is not made
 local to functions.
+
+Moreover, the warning and special exit code from
+tt([[ -o )var(non_existent_option)tt( ]]) are suppressed.
 )
 pindex(POSIX_IDENTIFIERS)
 pindex(NO_POSIX_IDENTIFIERS)
diff --git a/Src/cond.c b/Src/cond.c
index b9a47cea5..9f13e07d7 100644
--- a/Src/cond.c
+++ b/Src/cond.c
@@ -61,7 +61,8 @@ static void cond_subst(char **strp, int glob_ok)
  * of functionality.
  *
  * Return status is the final shell status, i.e. 0 for true,
- * 1 for false and 2 for error.
+ * 1 for false, 2 for syntax error, 3 for "option in tested in
+ * -o does not exist".
  */
 
 /**/
@@ -86,10 +87,10 @@ case COND_NOT:
 	if (tracingcond)
 	    fprintf(xtrerr, " %s", condstr[ctype]);
 	ret = evalcond(state, fromtest);
-	if (ret == 2)
-	    return ret;
-	else
+	if (ret == 0 || ret == 1)
 	    return !ret;
+	else
+	    return ret;
     case COND_AND:
 	if (!(ret = evalcond(state, fromtest))) {
 	    if (tracingcond)
@@ -100,7 +101,8 @@ case COND_AND:
 	    return ret;
 	}
     case COND_OR:
-	if ((ret = evalcond(state, fromtest)) == 1) {
+	ret = evalcond(state, fromtest);
+	if (ret == 1 || ret == 3) {
 	    if (tracingcond)
 		fprintf(xtrerr, " %s", condstr[ctype]);
 	    goto rec;
@@ -506,8 +508,12 @@ optison(char *name, char *s)
     else
 	i = optlookup(s);
     if (!i) {
-	zwarnnam(name, "no such option: %s", s);
-	return 2;
+	if (isset(POSIXBUILTINS))
+	    return 1;
+	else {
+	    zwarnnam(name, "no such option: %s", s);
+	    return 3;
+	}
     } else if(i < 0)
 	return !unset(-i);
     else
diff --git a/Test/C02cond.ztst b/Test/C02cond.ztst
index 38525016c..04e1ca8f2 100644
--- a/Test/C02cond.ztst
+++ b/Test/C02cond.ztst
@@ -440,6 +440,23 @@ F:Failures in these cases do not indicate a problem in the shell.
 >  [[ 'a' == 'b' || 'b' = 'c' || 'c' != 'd' ]]
 >}
 
+  (setopt posixbuiltins; eval '[[ -o invalidoption ]]; echo set: $?'; echo "no warning" >&2)
+  (unsetopt posixbuiltins; [[ -o invalidoption ]]; echo unset: $?)
+  [[ -o invalidoption || -n nonempty ]]; echo "in disjunction, true: $?"
+  [[ -o invalidoption || -z nonempty ]]; echo "in disjunction, false: $?"
+  [[ ! -o invalidoption ]]; echo "negated: $?"
+0:-o invalidoption
+>set: 1
+?no warning
+>unset: 3
+?(eval):2: no such option: invalidoption
+>in disjunction, true: 0
+?(eval):3: no such option: invalidoption
+>in disjunction, false: 1
+?(eval):4: no such option: invalidoption
+>negated: 3
+?(eval):5: no such option: invalidoption
+
 %clean
   # This works around a bug in rm -f in some versions of Cygwin
   chmod 644 unmodish