PATCH: edit-command-line: disable `monitor' option locally

PATCH: edit-command-line: disable `monitor' option locally

[Frank Terbeck]

I was just told, that suspending an editor while using
`edit-command-line' will cause the command line to be lost, yielding
this:

edit-command-line:zle:17: widgets can only be called when ZLE is active

Which makes sense.

I first thought to disable suspending by disabling the appropriate
shortcut via `stty'. But that doesn't work if the editor suspends anyway
when it sees a "C-z" byte (vim does).

So, in order to shield users from accidential dataloss, this patch
unsets the `monitor' option temporarily. That does the trick for me and
I don't think there is anything this would break.

If I'm missing something, yell.
---
 Functions/Zle/edit-command-line |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Functions/Zle/edit-command-line b/Functions/Zle/edit-command-line
index 250cac6..efcc4b9 100644
--- a/Functions/Zle/edit-command-line
+++ b/Functions/Zle/edit-command-line
@@ -6,6 +6,8 @@
 # will give ksh-like behaviour for that key,
 # except that it will handle multi-line buffers properly.
 
+setopt local_options no_monitor
+
 local tmpfile=${TMPPREFIX:-/tmp/zsh}ecl$$
 
 print -R - "$PREBUFFER$BUFFER" >$tmpfile
-- 
1.7.4.1.140.g89781

Re: PATCH: edit-command-line: disable `monitor' option locally

[Bart Schaefer]

On Mar 16,  9:16pm, Frank Terbeck wrote:
} Subject: PATCH: edit-command-line: disable `monitor' option locally
}
} I was just told, that suspending an editor while using
} `edit-command-line' will cause the command line to be lost, yielding
} this:
} 
} edit-command-line:zle:17: widgets can only be called when ZLE is active
} 
} Which makes sense.

Hmm.  I don't have this problem.  If I suspend from edit-command-line I
get brought back to the prompt and the previous input is all still there.
I can resume the editor with "fg" (at which point vim gives me an error
about the file no longer existing); if I write, exit, and then repeat
the edit-command-line keystroke, I'm back in the editor looking at the
stuff I wrote out a minute before, and if I now exit again that stuff
is loaded onto the command line.
 
} So, in order to shield users from accidential dataloss, this patch
} unsets the `monitor' option temporarily. That does the trick for me and
} I don't think there is anything this would break.

On the other hand if I put in this patch, then when I try to resume the
backgrounded editor I end up in a *very* strange state where the shell
is not responding to anything, and I have to go off to another window
and kill -CONT the editor before I can proceed.  In that case vim dies
almost immediately with "Vim: Error reading input, exiting..."
 
} If I'm missing something, yell.

I think perhaps this behaves very differently in different contexts.
Restartable system calls may be the difference, I don't know which
platform has the trouble that you described.

Re: PATCH: edit-command-line: disable `monitor' option locally

[Frank Terbeck]

Bart Schaefer wrote:
> On Mar 16,  9:16pm, Frank Terbeck wrote:
> } Subject: PATCH: edit-command-line: disable `monitor' option locally
> }
> } I was just told, that suspending an editor while using
> } `edit-command-line' will cause the command line to be lost, yielding
> } this:
> } 
> } edit-command-line:zle:17: widgets can only be called when ZLE is active
> } 
> } Which makes sense.
>
> Hmm.  I don't have this problem.

Hm. I had trouble reproducing this, too. So, I asked the guys for
precise instructions and they came up with something I could reproduce
as well.

> If I suspend from edit-command-line I get brought back to the prompt
> and the previous input is all still there.  I can resume the editor
> with "fg" (at which point vim gives me an error about the file no
> longer existing); if I write, exit, and then repeat the
> edit-command-line keystroke, I'm back in the editor looking at the
> stuff I wrote out a minute before, and if I now exit again that stuff
> is loaded onto the command line.

Here is how to get the error:

a) Start a new shell. The error seems to only appear the first time the
   `edit-command-line' widget is called.
b) Enter something into the command line buffer.
c) Strike your `edit-command-line' shortcut.
d) "C-z" to suspend.
e) Run another command while being suspended.
f) "fg" to get the editor back.
g) Exit the editor.

This leads to the error I mentioned.

Later attempts don't yield the error, but the command line buffer's
content is *gone* every time. Which to me is quite a grave problem, as
this may lead to rather huge multi-line commands to be gone.

> } So, in order to shield users from accidential dataloss, this patch
> } unsets the `monitor' option temporarily. That does the trick for me and
> } I don't think there is anything this would break.
>
> On the other hand if I put in this patch, then when I try to resume the
> backgrounded editor

Um. How do you end up with a backgrounded editor with this patch?
Shouldn't it be impossible to background the editor if `monitor' is
unset? I can't seem to get that to happen on my linux-based laptop.

> I end up in a *very* strange state where the shell is not responding
> to anything, and I have to go off to another window and kill -CONT the
> editor before I can proceed.  In that case vim dies almost immediately
> with "Vim: Error reading input, exiting..."
>  
> } If I'm missing something, yell.
>
> I think perhaps this behaves very differently in different contexts.
> Restartable system calls may be the difference, I don't know which
> platform has the trouble that you described.

This happens for me on various linux systems with different kernel
versions. I may be able to try OpenBSD later today, unless someone beats
me to it.


In any case, I think the widget should protect the user from losing the
command line. And disallowing the editor from backgrounding should do
that (and in fact does for me).

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: PATCH: edit-command-line: disable `monitor' option locally

[Bart Schaefer]

On Mar 17, 11:08am, Frank Terbeck wrote:
}
} Here is how to get the error:
} 
} a) Start a new shell. The error seems to only appear the first time the
}    `edit-command-line' widget is called.
} b) Enter something into the command line buffer.
} c) Strike your `edit-command-line' shortcut.
} d) "C-z" to suspend.
} e) Run another command while being suspended.
} f) "fg" to get the editor back.
} g) Exit the editor.

Nope, still nothing.  Even starting zsh with a ZDOTDIR pointing at a
file that does nothing but set up edit-command-line, I cannot make
this happen (with 4.3.11-dev-1 patchlevel 1.5223).

} > On the other hand if I put in this patch, then when I try to resume the
} > backgrounded editor
} 
} Um. How do you end up with a backgrounded editor with this patch?
} Shouldn't it be impossible to background the editor if `monitor' is
} unset? I can't seem to get that to happen on my linux-based laptop.

Here's with edit-command-line straight out of the distribution -- the
blank line is where I hit ^Z in the editor:

torch% echo something

zsh: suspended  
torch% echo something
torch% echo something

Now I hit enter and go on:

something
torch% jobs
[1]  + suspended  
torch% 

At this point I can bring vim back into the foreground with no trouble.
It's true the command line will be lost if I don't force vim to save it
somewhere, because the file has already been removed, the contents are
only in vim's working copy.

Now here's how it differs with your patch applied:

torch% echo otherthing

torch% echo otherthing
otherthing
torch% jobs
[1]  + suspended  
torch% 

I.e. the only thing that changed was whether I got notified about the
job being suspended.  Vim still handled the ^Z and stopped itself.
But bad things happen if I try to bring it back into the foreground
because zsh doesn't send CONT to the correct process group (I think
is what's going on, but I can't be sure).

} In any case, I think the widget should protect the user from losing the
} command line. And disallowing the editor from backgrounding should do
} that (and in fact does for me).

I agree but don't think twaddling no_monitor is sufficient.  If you
remove no_monitor but add xtrace to the setopts in edit-command-line,
what do you see after you hit ^Z in vim?  I get this:

----
torch% echo foo+edit-command-line:2> local 'tmpfile=/tmp/zshecl26567'
+edit-command-line:3> print -R - 'echo foo'
+edit-command-line:5> vi /tmp/zshecl26567


zsh: suspended  
torch% echo foo+edit-command-line:6> print -Rz - 'echo foo'
+edit-command-line:7> rm -f /tmp/zshecl26567
+edit-command-line:8> zle send-break

torch% echo foo
----

I suspect that in your case the entire edit-command-line function is
stopped, so rm -f / zle send-break never happens until after you bring
it into the foreground again, but I don't know why it differs for me.

Re: PATCH: edit-command-line: disable `monitor' option locally

[Frank Terbeck]

Bart Schaefer wrote:
> On Mar 17, 11:08am, Frank Terbeck wrote:
[...]
> } Um. How do you end up with a backgrounded editor with this patch?
> } Shouldn't it be impossible to background the editor if `monitor' is
> } unset? I can't seem to get that to happen on my linux-based laptop.
[...]
> Now here's how it differs with your patch applied:
>
> torch% echo otherthing
>
> torch% echo otherthing
> otherthing
> torch% jobs
> [1]  + suspended  
> torch% 
>
> I.e. the only thing that changed was whether I got notified about the
> job being suspended.  Vim still handled the ^Z and stopped itself.
> But bad things happen if I try to bring it back into the foreground
> because zsh doesn't send CONT to the correct process group (I think
> is what's going on, but I can't be sure).

Huh. That's quite bad. :-/

> } In any case, I think the widget should protect the user from losing the
> } command line. And disallowing the editor from backgrounding should do
> } that (and in fact does for me).
>
> I agree but don't think twaddling no_monitor is sufficient.  If you
> remove no_monitor but add xtrace to the setopts in edit-command-line,
> what do you see after you hit ^Z in vim?  I get this:
>
> ----
> torch% echo foo+edit-command-line:2> local 'tmpfile=/tmp/zshecl26567'
> +edit-command-line:3> print -R - 'echo foo'
> +edit-command-line:5> vi /tmp/zshecl26567
>
>
> zsh: suspended  
> torch% echo foo+edit-command-line:6> print -Rz - 'echo foo'
> +edit-command-line:7> rm -f /tmp/zshecl26567
> +edit-command-line:8> zle send-break
>
> torch% echo foo
> ----
>
> I suspect that in your case the entire edit-command-line function is
> stopped, so rm -f / zle send-break never happens until after you bring
> it into the foreground again, but I don't know why it differs for me.

Here it is:
[snip]
% zsh -f
zsh% cat t
autoload -Uz edit-command-line
zle -N edit-command-line
bindkey $'\ee' edit-command-line
zsh% source ./t
zsh% echo foobar+edit-command-line:11> local 'tmpfile=/tmp/zshecl16683'
+edit-command-line:13> print -R - 'echo foobar'
+edit-command-line:15> vim /tmp/zshecl16683
zsh: suspended  

zsh% fg
[1]  + continued  
+edit-command-line:16> print -Rz - 'echo foobar'
+edit-command-line:18> rm -f /tmp/zshecl16683
+edit-command-line:19> zle send-break
edit-command-line:zle:19: widgets can only be called when ZLE is active
[snap]


-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: PATCH: edit-command-line: disable `monitor' option locally

[Bart Schaefer]

On Mar 17,  6:56pm, Frank Terbeck wrote:
} Bart Schaefer wrote:
} > I suspect that in your case the entire edit-command-line function is
} > stopped, so rm -f / zle send-break never happens until after you bring
} > it into the foreground again, but I don't know why it differs for me.
} 
} Here it is:
} [snip]
} % zsh -f
} zsh% cat t
} autoload -Uz edit-command-line
} zle -N edit-command-line
} bindkey $'\ee' edit-command-line
} zsh% source ./t
} zsh% echo foobar+edit-command-line:11> local 'tmpfile=/tmp/zshecl16683'
} +edit-command-line:13> print -R - 'echo foobar'
} +edit-command-line:15> vim /tmp/zshecl16683
} zsh: suspended  

So, exactly as I thought, your entire shell function is being stopped
at this point.  This does not happen for me.  In fact, with VISUAL=less,
it becomes impossible for me to stop the "editor" AT ALL, even though
under normal circumstances I can suspend less normally.  Further, if
I set VISUAL=emacs so that it opens in a new window, I can't suspend
emacs by typing ^Z into the shell window.  This is with the ORIGINAL
edit-command-line, NOT with the patch to add no_monitor.

So AFAICT, for me, the only reason the ^Z is being handled at all is
because vim is handling it internally; monitor only affects whether
the shell does the right thing when vim does its own suspend.

If I add an   stty < /dev/tty >& /dev/tty   to edit-command-line, I
find that my susp character has been set to undef:

torch% VISUAL=cat
torch% +edit-command-line:2> local 'tmpfile=/tmp/zshecl26906'
+edit-command-line:3> print -R - ''
+edit-command-line:5> stty
speed 38400 baud; line = 0;
quit = <undef>; susp = <undef>; lnext = <undef>; flush = <undef>;
min = 1; time = 0;
-brkint inlcr -imaxbel
-icanon -echo
+edit-command-line:6> cat /tmp/zshecl26906

+edit-command-line:7> print -Rz - ''
+edit-command-line:8> rm -f /tmp/zshecl26906
+edit-command-line:9> zle send-break

torch% 

So zsh apparently is already trying to do the right thing.

What we need to figure out is why the suspend character has not been
successfully cleared for you (and for the people reporting this bug).
I believe it's zsetterm() in Src/Zle/zle_main.c that clears the VSUSP
field in the ttyinfo structure in my case.

Of course that doesn't help with vim handling the keystroke on its
own and pretending it got a signal that it never got.

-- 

Re: PATCH: edit-command-line: disable `monitor' option locally

[Frank Terbeck]

Bart Schaefer wrote:
[...]
> So zsh apparently is already trying to do the right thing.
>
> What we need to figure out is why the suspend character has not been
> successfully cleared for you (and for the people reporting this bug).
> I believe it's zsetterm() in Src/Zle/zle_main.c that clears the VSUSP
> field in the ttyinfo structure in my case.

I didn't have time to look at this in the required detail.  Should I
revert the change I made?  ...because evidently, it was not the right
thing[tm] to do.

Regards, Frank

Re: PATCH: edit-command-line: disable `monitor' option locally

[Bart Schaefer]

On Apr 25,  4:55pm, Frank Terbeck wrote:
}
} Bart Schaefer wrote:
} [...]
} > What we need to figure out is why the suspend character has not been
} > successfully cleared for you (and for the people reporting this bug).
} > I believe it's zsetterm() in Src/Zle/zle_main.c that clears the VSUSP
} > field in the ttyinfo structure in my case.
} 
} I didn't have time to look at this in the required detail.  Should I
} revert the change I made?  ...because evidently, it was not the right
} thing[tm] to do.

Hrm.  I had to look back through the thread ... the situation with
localoptions no_monitor is worse for me than without, and the whole
problem seems to come down to the editor stopping (sometimes itself)
without writing out its buffer, which is not something we can control
from within zsh.

I tried a few other things like putting the editor call in a while
loop, but I can't get that to behave any better.

So I reluctantly conclude that you ought to back out the change.

Re: PATCH: edit-command-line: disable `monitor' option locally

[Frank Terbeck]

Bart Schaefer wrote:
[...]
> the situation with localoptions no_monitor is worse for me than
> without
[...]
> So I reluctantly conclude that you ought to back out the change.

Agreed. [x] done.

Regards, Frank