PRINT_EXIT_VALUE: Suppress for if/while conditions

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

[tldr: question about printjob()]

I'd like to disable the effect of PRINT_EXIT_VALUE while evaluating
if/while conditions, since it's uninformative (conditions sometimes
fail, that's their sine qua non) and annoying (when doing a for/if
interactively and the 'if' condition is false in many iterations, the
option must be disabled to prevent stderr spamming).

So far I've got it working for builtins ("if false ; then : ; fi"
doesn't warn, whereas in git tip it does), but not for external commands
(with the patch, "if =false ; then : ; fi" still warns, but I'd like it
not to warn).  This is related to the MONITOR option:

    % if =false ; then : ; fi
    zsh: exit 1     =false
    % unsetopt monitor
    % if =false ; then : ; fi
    %

I'm guessing that has something to do with printjob(), since it checks
both 'jobbing' and opts[PRINTEXITVALUE], but I don't understand that
function.  Could I get a hint, please?

Would it be correct to just slip a "&& !printexitvalue_depth" to the "if
isset(PRINTEXITVALUE)" checks in printjob()?  I am not sure that would
be correct in the synch=0 case.

Thanks,

Daniel

P.S. I have a few other PRINT_EXIT_VALUE -related patches queued
my working drafts are at https://github.com/danielshahaf/zsh/commits/WIP/pev.
(I'll submit them as usual once they're ready for inclusion.)

From 6c04df26989fcf3c363fff43543110a1c323798e Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>
Date: Thu, 29 Jan 2015 12:34:19 +0000
Subject: [PATCH 1/4] Suppress PRINT_EXIT_VALUE for if and while conditions

The condition is *expected* to fail sometimes, so alerting the user to its
failure is not useful.
---
 Src/exec.c           |  6 ++++--
 Src/init.c           |  1 +
 Src/jobs.c           |  2 ++
 Src/loop.c           | 17 +++++++++++++++++
 Test/E01options.ztst |  4 ++++
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Src/exec.c b/Src/exec.c
index 7612d43..4c022cf 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -3620,7 +3620,8 @@ execcmd(Estate state, int input, int output, int how, int last1)
 		} else
 		    clearerr(stdout);
 	    }
-	    if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
+	    if (isset(PRINTEXITVALUE) && !printexitvalue_depth &&
+		isset(SHINSTDIN) &&
 		lastval && !subsh) {
 #if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
 		fprintf(stderr, "zsh: exit %lld\n", lastval);
@@ -4690,7 +4691,8 @@ execfuncdef(Estate state, Eprog redir_prog)
 	    execshfunc(shf, args);
 	    ret = lastval;
 
-	    if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
+	    if (isset(PRINTEXITVALUE) && !printexitvalue_depth &&
+		isset(SHINSTDIN) &&
 		lastval) {
 #if defined(ZLONG_IS_LONG_LONG) && defined(PRINTF_HAS_LLD)
 		fprintf(stderr, "zsh: exit %lld\n", lastval);
diff --git a/Src/init.c b/Src/init.c
index 2ef9099..bc2e459 100644
--- a/Src/init.c
+++ b/Src/init.c
@@ -1060,6 +1060,7 @@ setupvals(void)
 #endif
 
     breaks = loops = 0;
+    printexitvalue_depth = 0;
     lastmailcheck = time(NULL);
     locallevel = sourcelevel = 0;
     sfcontext = SFC_NONE;
diff --git a/Src/jobs.c b/Src/jobs.c
index a71df68..ae1acad 100644
--- a/Src/jobs.c
+++ b/Src/jobs.c
@@ -979,6 +979,7 @@ printjob(Job jn, int lng, int synch)
 		    sflag = 1;
 		if (job == thisjob && sig == SIGINT)
 		    doputnl = 1;
+		/* ### printexitvalue_depth? */
 		if (isset(PRINTEXITVALUE) && isset(SHINSTDIN)) {
 		    sflag = 1;
 		    skip_print = 0;
@@ -989,6 +990,7 @@ printjob(Job jn, int lng, int synch)
 		    len = strlen(sigmsg(sig));
 		if (job == thisjob && sig == SIGTSTP)
 		    doputnl = 1;
+	    /* ### printexitvalue_depth? */
 	    } else if (isset(PRINTEXITVALUE) && isset(SHINSTDIN) &&
 		       WEXITSTATUS(pn->status)) {
 		sflag = 1;
diff --git a/Src/loop.c b/Src/loop.c
index e4e8e2d..46622d7 100644
--- a/Src/loop.c
+++ b/Src/loop.c
@@ -35,6 +35,11 @@
 /**/
 int loops;
  
+/* nonzero if PRINTEXITVALUE is temporarily suppressed */
+
+/**/
+int printexitvalue_depth;
+
 /* # of continue levels */
  
 /**/
@@ -408,9 +413,14 @@ execwhile(Estate state, UNUSED(int do_exec))
     } else
         for (;;) {
             state->pc = loop;
+
+	    /* Evalute the condition. */
             noerrexit = 1;
+	    ++printexitvalue_depth;
             execlist(state, 1, 0);
+	    --printexitvalue_depth;
             noerrexit = olderrexit;
+
             if (!((lastval == 0) ^ isuntil)) {
                 if (breaks)
                     breaks--;
@@ -421,6 +431,7 @@ execwhile(Estate state, UNUSED(int do_exec))
                 lastval = oldval;
                 break;
             }
+	    /* Evaluate the body. */
             execlist(state, 1, 0);
             if (breaks) {
                 breaks--;
@@ -510,9 +521,14 @@ execif(Estate state, int do_exec)
 	    break;
 	}
 	next = state->pc + WC_IF_SKIP(code);
+
+	/* Evaluate the next condition. */
+	++printexitvalue_depth;
 	cmdpush(s ? CS_ELIF : CS_IF);
 	execlist(state, 1, 0);
 	cmdpop();
+	--printexitvalue_depth;
+
 	if (!lastval) {
 	    run = 1;
 	    break;
@@ -524,6 +540,7 @@ execif(Estate state, int do_exec)
     }
 
     if (run) {
+	/* Evaluate the body. */
 	/* we need to ignore lastval until we reach execcmd() */
 	noerrexit = olderrexit ? olderrexit : lastval ? 2 : 0;
 	cmdpush(run == 2 ? CS_ELSE : (s ? CS_ELIFTHEN : CS_IFTHEN));
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index 16279b8..c6a48e8 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -817,6 +817,10 @@
 # Goodness only knows why.
   $ZTST_testdir/../Src/zsh -f <<<'
       setopt printexitvalue
+      if false; then :; fi
+      while false; do :; done
+      if =false; then :; fi
+      while =false; do :; done
       func() {
 	  false
       }
-- 
2.1.4