Re: "{ } always { }" construct and return in called functions

Re: "{ } always { }" construct and return in called functions

[Mikael Magnusson]

On Wed, May 13, 2015 at 4:59 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Wed, 13 May 2015 14:31:41 +0200
> ia0 <zsh@ia0.eu> wrote:
>> There is something I don't understand with the "{ } always { }" construct.
>> What is the difference between the two following runs? I would expect the
>> last run to behave as the first one. What am I missing here?
>>
>> *% zsh --version*
>> zsh 5.0.2 (x86_64-pc-linux-gnu)
>> *% cat test*
>> #!/bin/zsh
>>
>> mytrue() { return 0 }
>> mywrap() { echo BEGIN; $1; echo END }
>> mytest() { { exit 1 } always { mywrap $1 } }
>>
>> mytest $1
>> *% ./test true*
>> BEGIN
>> END
>> *1% ./test mytrue*
>> BEGIN
>> *1% *
>
> Nested functions in always blocks with an exit pending are a corner case
> that needs fixing.  I'm not sure if this affects exit traps for
> functions, but possibly only in even weirder cases.
>
> Follow-ups can go to zsh-workers --- there are no user-serviceable
> parts inside.

This patch breaks my setup.

# Some hooks into accept-line
if [[ -n "$ZSHRUN" ]]; then
  unsetopt correct
  function _accept_and_quit() {
  local -a buf
  buf=(${(z)BUFFER})
  if which $buf[1] >& /dev/null; then
    zsh -c "${BUFFER}" &|
    exit
  else
    zle -M "Command $buf[1] not found"
  fi
  }
  zle -N _accept_and_quit
  bindkey "^M" _accept_and_quit
fi

With the patch, commands are randomly ran two or three times instead
of just once in the background. AFAIK there are no always blocks
involved in this part of my rc.

-- 
Mikael Magnusson

Re: "{ } always { }" construct and return in called functions

[Peter Stephenson]

On Mon, 18 May 2015 13:38:15 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> This patch breaks my setup.
> 
> # Some hooks into accept-line
> if [[ -n "$ZSHRUN" ]]; then
>   unsetopt correct
>   function _accept_and_quit() {
>   local -a buf
>   buf=(${(z)BUFFER})
>   if which $buf[1] >& /dev/null; then
>     zsh -c "${BUFFER}" &|
>     exit
>   else
>     zle -M "Command $buf[1] not found"
>   fi
>   }
>   zle -N _accept_and_quit
>   bindkey "^M" _accept_and_quit
> fi
> 
> With the patch, commands are randomly ran two or three times instead
> of just once in the background. AFAIK there are no always blocks
> involved in this part of my rc.

Are you actually trying to exit from the main shell here?  It claims to
be a hook into accept-line but it doesn't look to me like it is.

The key here is presumably sorting out what functions are running at
what level, i.e. where the "exit" command is first run and where the
shell actually exits (you could trying adding a message after "exit" to
see if that gets run, though that probably won't help since "retflag" is
likely to be set at that point).  In particular, what should happen is
the code at the end of doshfunc() causes the shell to exit at the end of
_accept_and_quit.  The simpler the case, the more likely
the relationship is to come out.

pws

Re: "{ } always { }" construct and return in called functions

[Mikael Magnusson]

On Mon, May 18, 2015 at 2:14 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Mon, 18 May 2015 13:38:15 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> This patch breaks my setup.
>>
>> # Some hooks into accept-line
>> if [[ -n "$ZSHRUN" ]]; then
>>   unsetopt correct
>>   function _accept_and_quit() {
>>   local -a buf
>>   buf=(${(z)BUFFER})
>>   if which $buf[1] >& /dev/null; then
>>     zsh -c "${BUFFER}" &|
>>     exit
>>   else
>>     zle -M "Command $buf[1] not found"
>>   fi
>>   }
>>   zle -N _accept_and_quit
>>   bindkey "^M" _accept_and_quit
>> fi
>>
>> With the patch, commands are randomly ran two or three times instead
>> of just once in the background. AFAIK there are no always blocks
>> involved in this part of my rc.
>
> Are you actually trying to exit from the main shell here?  It claims to
> be a hook into accept-line but it doesn't look to me like it is.

Yes, I have a slightly alternate codepath through my .zshrc triggered
by ZSHRUN being set in the env, used for a Win+r shortcut to run
simple commands in the background. The comment is probably more
accurate for the hook defined below that, used for the normal main
shell case.

> The key here is presumably sorting out what functions are running at
> what level, i.e. where the "exit" command is first run and where the
> shell actually exits (you could trying adding a message after "exit" to
> see if that gets run, though that probably won't help since "retflag" is
> likely to be set at that point).  In particular, what should happen is
> the code at the end of doshfunc() causes the shell to exit at the end of
> _accept_and_quit.  The simpler the case, the more likely
> the relationship is to come out.

I would have expected the shell to exit when encountering the "exit"
command, and run no further commands, no matter the circumstances.
Probably a naive viewpoint considering all I've seen. I've reverted
the patch locally for now, but I might investigate more later then.

-- 
Mikael Magnusson

Re: "{ } always { }" construct and return in called functions

[Peter Stephenson]

On Mon, 18 May 2015 14:24:51 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I would have expected the shell to exit when encountering the "exit"
> command, and run no further commands, no matter the circumstances.
> Probably a naive viewpoint considering all I've seen. I've reverted
> the patch locally for now, but I might investigate more later then.

bin_break(), which handles exit, says:

    case BIN_EXIT:
	if (locallevel > forklevel) {
	    /*
	     * We don't exit directly from functions to allow tidying
	     * up, in particular EXIT traps.  We still need to perform
	     * the usual interactive tests to see if we can exit at
	     * all, however.
	     *
	     * If we are forked, we exit the shell at the function depth
	     * at which we became a subshell, hence the comparison.
	     */
	    if (stopmsg || (zexit(0,2), !stopmsg)) {
		retflag = 1;
		breaks = loops;
		exit_pending = (num << 1) | 1;
		exit_level = locallevel;
	    }
	} else
	    zexit(num, 0);
	break;

"always" blocks are another form of tidy up that may or may not apply:
we're too deep here to know whether they do or not.

But there's nothing in the code in doshfunc() that should prevent the exit
happening there, unless there's some fork()ing going on that isn't
obvious.  We need to work out what path it's taking through both chunks
of code (some fprintf's might be good enough).

Hmmm...  I'm wondering if the fact you've started jobs is important.  It
shouldn't be, with the "&|", and the doshfunc() code is supposed
to make it irrelevant anyway by setting "stopmsg = 1".

pws

Re: "{ } always { }" construct and return in called functions

[Bart Schaefer]

On May 18,  1:47pm, Peter Stephenson wrote:
} Subject: Re: "{ } always { }" construct and return in called functions
}
} Hmmm...  I'm wondering if the fact you've started jobs is important.  It
} shouldn't be, with the "&|", and the doshfunc() code is supposed
} to make it irrelevant anyway by setting "stopmsg = 1".

If you have some other job running in the background, the shell does
not exit when 'exit' is called from a zle widget:

torch%   unsetopt correct
torch%   function _accept_and_quit() {
function>   local -a buf
function>   buf=(${(z)BUFFER})
function>   if which $buf[1] >& /dev/null; then
function then>     zsh -c "${BUFFER}" &|
function then>     exit
function then>   else
function else>     zle -M "Command $buf[1] not found"
function else>   fi
function>   }
torch%   zle -N _accept_and_quit
torch% { echo BG: $RANDOM; sleep 30; } &
[1] 7442
BG: 12801                                                                       
torch% bindkey "^M" _accept_and_quit
torch% { echo Here: $RANDOM; sleep 10 }
_accept_and_quit:5: you have running jobs.
torch% { echo Here: $RANDOM; sleep 10 }
torch% Here: 29846
echo not exited
torch% 
zsh: warning: 1 jobs SIGHUPed


It's the same effect as this:

torch% { sleep 30 } &
[1] 7457
torch% exit
zsh: you have running jobs.
torch% exit
zsh: warning: 1 jobs SIGHUPed

The difference is that the "you have running jobs" message is suppressed
when exit is called from a zle widget.

If Mikael adds "unsetopt checkjobs" he should get the effect he wants.
He might also want "setopt nohup" to prevent the death of whatever it
is that's keeping the shell alive.

The part about "commands are randomly ran two or three times" does not
happen for me -- though I don't know exactly what it means.  Does it
mean you can invoke _accept_and_quit two or three times, or does it
mean that running _accept_and_quit once mysteriously launches the same
job two or three times?

Re: "{ } always { }" construct and return in called functions

[Mikael Magnusson]

On Tue, May 19, 2015 at 7:04 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On May 18,  1:47pm, Peter Stephenson wrote:
> } Subject: Re: "{ } always { }" construct and return in called functions
> }
> } Hmmm...  I'm wondering if the fact you've started jobs is important.  It
> } shouldn't be, with the "&|", and the doshfunc() code is supposed
> } to make it irrelevant anyway by setting "stopmsg = 1".
>
> If you have some other job running in the background, the shell does
> not exit when 'exit' is called from a zle widget:
>
> torch%   unsetopt correct
> torch%   function _accept_and_quit() {
> function>   local -a buf
> function>   buf=(${(z)BUFFER})
> function>   if which $buf[1] >& /dev/null; then
> function then>     zsh -c "${BUFFER}" &|
> function then>     exit
> function then>   else
> function else>     zle -M "Command $buf[1] not found"
> function else>   fi
> function>   }
> torch%   zle -N _accept_and_quit
> torch% { echo BG: $RANDOM; sleep 30; } &
> [1] 7442
> BG: 12801
> torch% bindkey "^M" _accept_and_quit
> torch% { echo Here: $RANDOM; sleep 10 }
> _accept_and_quit:5: you have running jobs.
> torch% { echo Here: $RANDOM; sleep 10 }
> torch% Here: 29846
> echo not exited
> torch%
> zsh: warning: 1 jobs SIGHUPed
>
>
> It's the same effect as this:
>
> torch% { sleep 30 } &
> [1] 7457
> torch% exit
> zsh: you have running jobs.
> torch% exit
> zsh: warning: 1 jobs SIGHUPed
>
> The difference is that the "you have running jobs" message is suppressed
> when exit is called from a zle widget.
>
> If Mikael adds "unsetopt checkjobs" he should get the effect he wants.
> He might also want "setopt nohup" to prevent the death of whatever it
> is that's keeping the shell alive.

A good theory, but
zshrun ~> echo $options[checkjobs]
off
I've always had that option turned off.

Also, since the command is run with &| then surely there are no jobs either way?

> The part about "commands are randomly ran two or three times" does not
> happen for me -- though I don't know exactly what it means.  Does it
> mean you can invoke _accept_and_quit two or three times, or does it
> mean that running _accept_and_quit once mysteriously launches the same
> job two or three times?

It seems to be run once in the background by the widget, and then
another one in the foreground that I can ctrl-c out of. I only press
enter the one time.

-- 
Mikael Magnusson

Re: "{ } always { }" construct and return in called functions

[Mikael Magnusson]

On Tue, May 19, 2015 at 8:22 AM, Mikael Magnusson <mikachu@gmail.com> wrote:
> On Tue, May 19, 2015 at 7:04 AM, Bart Schaefer
> <schaefer@brasslantern.com> wrote:
>> On May 18,  1:47pm, Peter Stephenson wrote:
>> } Subject: Re: "{ } always { }" construct and return in called functions
>> }
>> } Hmmm...  I'm wondering if the fact you've started jobs is important.  It
>> } shouldn't be, with the "&|", and the doshfunc() code is supposed
>> } to make it irrelevant anyway by setting "stopmsg = 1".
>>
>> If you have some other job running in the background, the shell does
>> not exit when 'exit' is called from a zle widget:
>>
>> torch%   unsetopt correct
>> torch%   function _accept_and_quit() {
>> function>   local -a buf
>> function>   buf=(${(z)BUFFER})
>> function>   if which $buf[1] >& /dev/null; then
>> function then>     zsh -c "${BUFFER}" &|
>> function then>     exit
>> function then>   else
>> function else>     zle -M "Command $buf[1] not found"
>> function else>   fi
>> function>   }
>> torch%   zle -N _accept_and_quit
>> torch% { echo BG: $RANDOM; sleep 30; } &
>> [1] 7442
>> BG: 12801
>> torch% bindkey "^M" _accept_and_quit
>> torch% { echo Here: $RANDOM; sleep 10 }
>> _accept_and_quit:5: you have running jobs.
>> torch% { echo Here: $RANDOM; sleep 10 }
>> torch% Here: 29846
>> echo not exited
>> torch%
>> zsh: warning: 1 jobs SIGHUPed
>>
>>
>> It's the same effect as this:
>>
>> torch% { sleep 30 } &
>> [1] 7457
>> torch% exit
>> zsh: you have running jobs.
>> torch% exit
>> zsh: warning: 1 jobs SIGHUPed
>>
>> The difference is that the "you have running jobs" message is suppressed
>> when exit is called from a zle widget.
>>
>> If Mikael adds "unsetopt checkjobs" he should get the effect he wants.
>> He might also want "setopt nohup" to prevent the death of whatever it
>> is that's keeping the shell alive.
>
> A good theory, but
> zshrun ~> echo $options[checkjobs]
> off
> I've always had that option turned off.
>
> Also, since the command is run with &| then surely there are no jobs either way?
>
>> The part about "commands are randomly ran two or three times" does not
>> happen for me -- though I don't know exactly what it means.  Does it
>> mean you can invoke _accept_and_quit two or three times, or does it
>> mean that running _accept_and_quit once mysteriously launches the same
>> job two or three times?
>
> It seems to be run once in the background by the widget, and then
> another one in the foreground that I can ctrl-c out of. I only press
> enter the one time.

If I comment out the exit in my widget, then the command is only run
once, and the commandline stays as it is.

If I put set -x at the head of the function, I get this output when
pressing enter with also the zsh -c line commented out first:
+_accept_and_quit:2> local -a buf
+_accept_and_quit:3> buf=( urxvt )
+_accept_and_quit:4> which urxvt
+_accept_and_quit:6> exit

when the last line is printed, urxvt is run in the foreground. After
exiting that terminal, our zsh exits.

-- 
Mikael Magnusson

Re: "{ } always { }" construct and return in called functions

[Bart Schaefer]

On May 19,  8:22am, Mikael Magnusson wrote:
} Subject: Re: "{ } always { }" construct and return in called functions
}
} On Tue, May 19, 2015 at 7:04 AM, Bart Schaefer
} > If you have some other job running in the background, the shell does
} > not exit when 'exit' is called from a zle widget:
} 
} A good theory, but
} zshrun ~> echo $options[checkjobs]
} off
} I've always had that option turned off.
} 
} Also, since the command is run with &| then surely there are no jobs
} either way?

I was presuming that whatever other job was preventing the exit had been
started somewhere earlier in the startup sequence, and was not the same
job started by _accept_and_quit.

} It seems to be run once in the background by the widget, and then
} another one in the foreground that I can ctrl-c out of. I only press
} enter the one time.

This makes me suspect that both _accept_and_quit and also accept-line
are being executed.

Perhaps the shell is receiving "\r\n" and executing _accept_and_quit on
the \r followed by accept-line on the \n ?

Try

    zsh -c "${BUFFER}" &|
    BUFFER=""
    exit

??

Re: "{ } always { }" construct and return in called functions

[Peter Stephenson]

On Mon, 18 May 2015 13:38:15 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> This patch breaks my setup.
> 
>   function _accept_and_quit() {
>   local -a buf
>   buf=(${(z)BUFFER})
>   if which $buf[1] >& /dev/null; then
>     zsh -c "${BUFFER}" &|
>     exit
>   else
>     zle -M "Command $buf[1] not found"
>   fi
>   }
>   zle -N _accept_and_quit
>   bindkey "^M" _accept_and_quit
> fi
> 
> With the patch, commands are randomly ran two or three times instead
> of just once in the background.

The story appears to be someone decided there should be a double
start/end parameter scope for reasons I'm scrupulously going to avoid
investigating (it's not new), so the locallevel jumps.  So the new test
never triggers.  Adding a ">" puts this back how it was.

This is the last thing I have to look at; is anyone else expecting
something to happen?

pws

diff --git a/Src/exec.c b/Src/exec.c
index 527dffb..527d611 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -5109,7 +5109,7 @@ doshfunc(Shfunc shfunc, LinkList doshargs, int noreturnval)
      * when we have an "always" block.  The endparamscope() has
      * already happened, hence the "+1" here.
      */
-    if (exit_pending && exit_level == locallevel+1) {
+    if (exit_pending && exit_level >= locallevel+1) {
 	if (locallevel > forklevel) {
 	    /* Still functions to return: force them to do so. */
 	    retflag = 1;

Re: "{ } always { }" construct and return in called functions

[Bart Schaefer]

On May 22, 11:31pm, Peter Stephenson wrote:
}
} This is the last thing I have to look at; is anyone else expecting
} something to happen?

I can't think of anything else, and as is traditional for at least
one of us each release, I'm going to be unavailable to work on
anything for the next several days, so you may as well proceed.

Re: "{ } always { }" construct and return in called functions

[Daniel Shahaf]

Peter Stephenson wrote on Fri, May 22, 2015 at 23:31:13 +0100:
> This is the last thing I have to look at; is anyone else expecting
> something to happen?

[ I assume you're asking if there are release blockers. ]

I *think* the compdescribe stuff is fine.  All of the issues discussed
in that thread are either not new or not happening in master.  My main
concern is that 35127#1 might have introduced a bug that hasn't been
noticed yet.  I'm more concerned about that patch than usual since it is
my first patch to comp*.c and I'm not at home in that part of the code, yet.

Cheers,

Daniel

Re: "{ } always { }" construct and return in called functions

[Mikael Magnusson]

On Sat, May 23, 2015 at 12:31 AM, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> This is the last thing I have to look at; is anyone else expecting
> something to happen?

I uploaded the current build to Coverity, and it found nothing new to
complain about.

-- 
Mikael Magnusson

Re: "{ } always { }" construct and return in called functions

[Dave Yost]

Here’s one for you:

By accident I often type !4 and then tab 
instead of !$ and then tab.
When I backspace over the !4 and type !$, the !$ seems to be invisible, i.e. doesn’t work.

Bang-four (Re: "{ } always { }" construct and return in called functions)

[Bart Schaefer]

On May 26,  7:20pm, Dave Yost wrote:
} 
} By accident I often type !4 and then tab 
} instead of !$ and then tab.
} When I backspace over the !4 and type !$, the !$ seems to be invisible, i.e. doesn't work.

This happens because when !4 is expanded, it sets the current history event
to 4.  Then when you expand !$, it tries to grab the last word of event #4.
If your shell has been running long enough that event #4 has fallen out of
the history, there is no last word of that event to expand.

This has always bothered me a bit -- it sort of makes sense that when you
type out a command line containing !4 !$ and hit enter, you get the same
history event for both references, but when expanding one at a time with
ZLE's expand-word that makes much less sense.  On the other hand if you
tab-expand the entire line at once, you do want the same behavior as if
you hit enter.

Anyway you can make all of this sane with "setopt CSH_JUNKIE_HISTORY".

Release blocker (was: Re: "{ } always { }" construct and return in called functions)

[Daniel Hahler]

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

On 23.05.2015 00:31, Peter Stephenson wrote:

> This is the last thing I have to look at; is anyone else expecting
> something to happen?

For a release?

I think 35268 (Regression with completion cache) should be considered to be a blocker.


Regards,
Daniel.

-- 
http://daniel.hahler.de/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 173 bytes --]

Re: Release blocker (was: Re: "{ } always { }" construct and return in called functions)

[Peter Stephenson]

On Sun, 24 May 2015 00:32:47 +0200
Daniel Hahler <genml+zsh-workers@thequod.de> wrote:
> On 23.05.2015 00:31, Peter Stephenson wrote:
> 
> > This is the last thing I have to look at; is anyone else expecting
> > something to happen?
> 
> For a release?
> 
> I think 35268 (Regression with completion cache) should be considered
> to be a blocker.

Yes, as that's an optimisation it might be best to back it off for now;
then Bart can fix it at leisure.

pws

Re: Release blocker (was: Re: "{ } always { }" construct and return in called functions)

[Peter Stephenson]

On Sun, 24 May 2015 13:54:05 +0100
Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Sun, 24 May 2015 00:32:47 +0200
> Daniel Hahler <genml+zsh-workers@thequod.de> wrote:
> > On 23.05.2015 00:31, Peter Stephenson wrote:
> > 
> > > This is the last thing I have to look at; is anyone else expecting
> > > something to happen?
> > 
> > For a release?
> > 
> > I think 35268 (Regression with completion cache) should be considered
> > to be a blocker.
> 
> Yes, as that's an optimisation it might be best to back it off for now;
> then Bart can fix it at leisure.

I've done that --- I'm not going to get a chance to produce a new
release candidate before tomorrow evening but as there have been some
significant changes I'll produce another dev version for what I hope
will be some quick checks.

pws