From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: Peter Stephenson <p.stephenson@samsung.com>
Cc: zsh-workers@zsh.org
Subject: Re: PRINT_EXIT_VALUE: Suppress for if/while conditions
Date: Thu, 13 Aug 2015 23:20:20 +0000 [thread overview]
Message-ID: <20150813232020.GF1998@tarsus.local2> (raw)
In-Reply-To: <20150813093238.046a1b08@pwslap01u.europe.root.pri>
Peter Stephenson wrote on Thu, Aug 13, 2015 at 09:32:38 +0100:
> On Fri, 31 Jul 2015 23:12:25 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> > 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?
>
> It's a bit mysterious quite why it's implemented like that --- you might
> have thought something parallel to ERREXIT would make more sense --- but
> I don'e think what it actually does is that mysterious. Bascially,
> handling of printing exitvalues is divided into two parts: for anything
> that runs within the shell it's done immediately the command is run; for
> anything else it runs in printjob() when the job status changes (with
> the side effect of dependence on MONITOR). It might be done this way to
> handle background jobs, which wouldn't be picked up if you did it in a
> way more naturally related to the execution hierarchy.
>
> But the intention is clearly that these are otherwise parallel cases.
> So I think anything you do in the one case you can do in the other, as
> you're proposing, up to asynchronous effects.
>
> > 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.
>
> Yes, I think you probably need something like
>
> (!printexitvalue_depth && sync == 1)
>
> since the cases of calling it from jobs or bg or fg are irrelevant.
First of all, thanks for having a look.
I have no problem with ruling out the jobs/bg/fg callsites, as you
propose. However, checking (synch == 1) would also mean the value of
PRINTEXITVALUE is entirely ignored when printjob() is called
asynchronously. I can see that that is fine for jobs that don't have
STAT_NOPRINT set.¹ Is it also correct to ignore PRINTEXITVALUE in the
case (synch == 0 && (jn->stat & STAT_NOPRINT))?
(I'm guessing the answer is affirmative, and that if it were negative
I should be testing the value of printexitvalue_depth that was current
when the background job was started.)
In any case, I think I'll wait with this series until after 5.0.9, to
give it as long as possible to stabilize.
Cheers,
Daniel
¹ For jobs without STAT_NOPRINT, the only thing the 'if PRINTEXITVALUE'
block does is set sflag; the only place that tests sflag is a '(sflag ||
job != thisjob)' condition; for asynchronous jobs, the value of this
condition is independent of the value of 'sflag' (the disjunction would
be true because the inequality would be true). Therefore,
non-STAT_NOPRINT jobs would be printed anyway, regardless of the setting
of PRINTEXITVALUE.
> pws
>
> pws
next prev parent reply other threads:[~2015-08-13 23:20 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-31 23:12 Daniel Shahaf
2015-08-13 8:32 ` Peter Stephenson
2015-08-13 23:20 ` Daniel Shahaf [this message]
2015-08-14 8:19 ` Peter Stephenson
2015-08-17 22:00 ` Daniel Shahaf
2015-08-15 1:04 ` Vincent Lefevre
2015-08-15 1:20 ` Mikael Magnusson
2015-08-19 0:06 ` Vincent Lefevre
2015-08-19 0:25 ` Bart Schaefer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150813232020.GF1998@tarsus.local2 \
--to=d.s@daniel.shahaf.name \
--cc=p.stephenson@samsung.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).