On Thu, Dec 1, 2022 at 6:42 AM Philippe Altherr
<philippe.altherr@gmail.com> wrote:
>
> https://zsh.sourceforge.io/Doc/Release/Options.html#index-ERRRETURN.
Will that doc remain correct with respect to what it says about
ERR_EXIT after recent patches are considered?
(Still on my TODO list, have been busier than I expected so far this week.)
[-- Attachment #1: Type: text/plain, Size: 844 bytes --] Yep, that remains correct. Same for ERR_EXIT. Most(/All?) of the patches address cases where the implementation wasn't following the specification. The patch about the always statement makes it behave like compound statements. Since the always statement isn't part of POSIX, this could be seen either as a bug fix or a spec change (but rather of the always statement than of ERR_EXIT). Philippe On Thu, Dec 1, 2022 at 5:30 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > On Thu, Dec 1, 2022 at 6:42 AM Philippe Altherr > <philippe.altherr@gmail.com> wrote: > > > > https://zsh.sourceforge.io/Doc/Release/Options.html#index-ERRRETURN. > > Will that doc remain correct with respect to what it says about > ERR_EXIT after recent patches are considered? > > (Still on my TODO list, have been busier than I expected so far this week.) > [-- Attachment #2: Type: text/html, Size: 1441 bytes --]
On Thu, Dec 1, 2022 at 11:30 AM Philippe Altherr
<philippe.altherr@gmail.com> wrote:
>
> The patch about the always statement makes it behave like compound statements.
A statement followed by an always-block should always behave as if the
always-block wasn't there, except in the TRY_BLOCK_ERROR cases
mentioned in the description of "always". I suppose your patches
should be tested against that.
[-- Attachment #1: Type: text/plain, Size: 987 bytes --] The following script prints "done 1" set -e > false && true > echo done $? If a statement followed by an always-block always behaves as if the always-block wasn't there, then the following script should print the same (after "ALWAYS") set -e > { false && true } always { echo ALWAYS } > echo done $? The current Zsh only prints "ALWAYS", while the patched one prints "ALWAYS" followed by "done 1". So I guess that even the always patch can be qualified as a bug fix. Philippe On Thu, Dec 1, 2022 at 9:05 PM Bart Schaefer <schaefer@brasslantern.com> wrote: > On Thu, Dec 1, 2022 at 11:30 AM Philippe Altherr > <philippe.altherr@gmail.com> wrote: > > > > The patch about the always statement makes it behave like compound > statements. > > A statement followed by an always-block should always behave as if the > always-block wasn't there, except in the TRY_BLOCK_ERROR cases > mentioned in the description of "always". I suppose your patches > should be tested against that. > [-- Attachment #2: Type: text/html, Size: 1855 bytes --]
On 29 Nov, Roman Perepelitsa wrote: > I have the following in my .zshrc: > > function skip-csi-sequence() { > local key > while read -sk key && (( $((#key)) < 0x40 || $((#key)) > 0x7E )); do > # empty body > done > } > > zle -N skip-csi-sequence > bindkey '\e[' skip-csi-sequence > > With this binding a buggy script that leaks mouse events into zle > won't have any effect. This is a nice idea. I've occasionally had unwanted effects from unintentionally hitting a function key. I thought I'd try this in C. After testing this, I'm inclined to think it would be better to special case the CSI sequence in getkeymapcmd() instead. Currently, you would need to bind this in every local keymap because most local keymaps will include a binding starting with the \e[ sequence. It is also common for Alt-[ to generate \e[. Binding that would override the skip-csi-sequence binding but we could discern between the two based on KEYTIMEOUT. Any other thoughts on this. Or objections to such an approach? Are there terminals out there that eschew CSI sequences and for which such special handling could be problematic? Is anyone especially attached to the current behaviour where unbound function keys and mouse movements cause chaos? I notice that bash has a skip-csi-sequence widget but leaves it disabled by default. Would be interesting ot know why. The patch below tries the widget approach. Rather than look for the termination character, I thought it better to bail out given any out-of-range parameter byte or KEYTIMEOUT being exceeded. As yet, I'm not planning to apply this. Oliver diff --git a/Src/Zle/iwidgets.list b/Src/Zle/iwidgets.list index c95c7a491..8d1b64e58 100644 --- a/Src/Zle/iwidgets.list +++ b/Src/Zle/iwidgets.list @@ -112,6 +112,7 @@ "self-insert-unmeta", selfinsertunmeta, ZLE_MENUCMP | ZLE_KEEPSUFFIX "send-break", sendbreak, 0 "set-mark-command", setmarkcommand, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL +"skip-csi-sequence", skipcsisequence, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL "split-undo", splitundo, ZLE_MENUCMP | ZLE_KEEPSUFFIX | ZLE_LASTCOL | ZLE_NOTCOMMAND "spell-word", spellword, 0 "set-local-history", setlocalhistory, ZLE_LASTCOL diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c index d90838f03..737d3cd72 100644 --- a/Src/Zle/zle_keymap.c +++ b/Src/Zle/zle_keymap.c @@ -1430,6 +1430,11 @@ default_bindings(void) bindkey(emap, "\30\30", refthingy(t_exchangepointandmark), NULL); bindkey(emap, "\30=", refthingy(t_whatcursorposition), NULL); + /* skip unbound CSI sequence in all keymaps */ + bindkey(emap, "\33[", refthingy(t_skipcsisequence), NULL); + bindkey(vmap, "\33[", refthingy(t_skipcsisequence), NULL); + bindkey(amap, "\33[", refthingy(t_skipcsisequence), NULL); + /* bracketed paste applicable to all keymaps */ bindkey(emap, "\33[200~", refthingy(t_bracketedpaste), NULL); bindkey(vmap, "\33[200~", refthingy(t_bracketedpaste), NULL); diff --git a/Src/Zle/zle_misc.c b/Src/Zle/zle_misc.c index eba28d1ec..1b996fb8a 100644 --- a/Src/Zle/zle_misc.c +++ b/Src/Zle/zle_misc.c @@ -152,6 +152,19 @@ selfinsertunmeta(char **args) return selfinsert(args); } + +/**/ +mod_export int +skipcsisequence(char **args) +{ + int next, timeout; + do { + next = getbyte(1L, &timeout, 1); + } while (!timeout && next >= 0x20 && next <= 0x3f); + + return 1; +} + /**/ int deletechar(char **args)
On Mon, Dec 5, 2022 at 3:49 PM Oliver Kiddle <opk@zsh.org> wrote:
>
> After testing this, I'm inclined to think it would be better to special
> case the CSI sequence in getkeymapcmd() instead. [...]
> Is anyone especially attached to the
> current behaviour where unbound function keys and mouse movements cause
> chaos?
I'm not attached to them causing chaos, but I'm not sure discarding
them unacknowledged is the right thing either. Roman's binding does
produce some unexpected behavior if you actually type escape bracket
(which might occur in vi modes, if unlikely in emacs?). Some kind of
feep or "zle -M"-style warning, or is that also chaos?
On Tue, Dec 6, 2022 at 12:48 AM Oliver Kiddle <opk@zsh.org> wrote: > > On 29 Nov, Roman Perepelitsa wrote: > > I have the following in my .zshrc: > > > > function skip-csi-sequence() { > > local key > > while read -sk key && (( $((#key)) < 0x40 || $((#key)) > 0x7E )); do > > # empty body > > done > > } > > > > zle -N skip-csi-sequence > > bindkey '\e[' skip-csi-sequence > > > > With this binding a buggy script that leaks mouse events into zle > > won't have any effect. > > This is a nice idea. I've occasionally had unwanted effects from > unintentionally hitting a function key. That's its intended purpose. I learned about readline's skip-csi-sequence here: https://www.reddit.com/r/zsh/comments/yzhx3l. Docs: https://www.gnu.org/software/bash/manual/html_node/Miscellaneous-Commands.html#index-skip_002dcsi_002dsequence-_0028_0029 """ skip-csi-sequence () Read enough characters to consume a multi-key sequence such as those defined for keys like Home and End. Such sequences begin with a Control Sequence Indicator (CSI), usually ESC-[. If this sequence is bound to "\e[", keys producing such sequences will have no effect unless explicitly bound to a Readline command, instead of inserting stray characters into the editing buffer. This is unbound by default, but usually bound to ESC-[. """ > The patch below tries the widget approach. Rather than look for the > termination character, I thought it better to bail out given any > out-of-range parameter byte [...] Good idea. This is also how readline does it. > [...] or KEYTIMEOUT being exceeded Probably also a good idea, although readline doesn't have a timeout. Is there an advantage to implementing this widget in C rather than zsh? Roman.
Bart wrote: > I'm not attached to them causing chaos, but I'm not sure discarding > them unacknowledged is the right thing either. Roman's binding does > produce some unexpected behavior if you actually type escape bracket > (which might occur in vi modes, if unlikely in emacs?). Some kind of > feep or "zle -M"-style warning, or is that also chaos? Returning 1 from the widget function as I did in the C implementation does result in a feep. Allowing for KEYTIMEOUT resolves unexpected behaviour if you actually type escape [. Roman wrote: > Is there an advantage to implementing this widget in C rather than zsh? If implemented in shell code, every user that wants to make use of it needs to setup the function and bind it for every keymap. That includes local keymaps given that it is near certain to be a prefix of other key bindings. What advantage do you see to implementing it in shell code? I wrote: > After testing this, I'm inclined to think it would be better to special > case the CSI sequence in getkeymapcmd() instead. The new patch below takes this approach. This is able to be even stricter on the definition of a CSI sequence. This has the advantage of working even where the key sequence doesn't start with a CSI sequence. So if Ctrl-X is a prefix, you can press Ctrl-X followed by Delete and it'll wait for the full CSI sequence from the Delete key. Similarly for cases like Alt-Delete generating a sequence like '^[^[[3~'. If you examine keys with describe-key-briefly, it will now give, e.g "^[[12~" is undefined-key for F2 where before I got "^[" is vi-cmd-suffix-retain and [12~ inserted. undefined-key is implemented as just return 1 so this results in a feep but with mouse events enabled, that's all that happens. You still don't want to enable mouse events if you've not arranged for them to be handled. Oliver diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c index d90838f03..2fe48796c 100644 --- a/Src/Zle/zle_keymap.c +++ b/Src/Zle/zle_keymap.c @@ -1586,7 +1586,7 @@ getkeymapcmd(Keymap km, Thingy *funcp, char **strp) Thingy func = t_undefinedkey; char *str = NULL; int lastlen = 0, lastc = lastchar; - int timeout = 0; + int timeout = 0, csi = 0, startcsi; keybuflen = 0; keybuf[0] = 0; @@ -1636,7 +1636,26 @@ getkeymapcmd(Keymap km, Thingy *funcp, char **strp) } #endif } - if (!ispfx) + + /* CSI key sequences have a well defined structure so if we currently + * have an incomplete one, loop so the rest of it will be included in + * the key sequence if that arrives within the timeout. */ + if (keybuflen >= 3 && !csi) { + startcsi = keybuflen - 3; + csi = keybuf[startcsi] == '\033' && keybuf[keybuflen - 2] == '['; + } + if (csi) { + csi = keybuf[keybuflen - 2] != Meta && keybuf[keybuflen - 1] >= 0x20 + && keybuf[keybuflen - 1] <= 0x3f; + if (!csi && keybuf[keybuflen - 1] >= 0x40 && + keybuf[keybuflen - 1] <= 0x7e && lastlen > startcsi && + lastlen != keybuflen) { + func = t_undefinedkey; + lastlen = keybuflen; + } + } + + if (!ispfx && !csi) break; } if(!lastlen && keybuflen)
On Sat, Dec 10, 2022 at 2:04 AM Oliver Kiddle <opk@zsh.org> wrote:
>
> Roman wrote:
> > Is there an advantage to implementing this widget in C rather than zsh?
>
> If implemented in shell code, every user that wants to make use of it
> needs to setup the function and bind it for every keymap. That includes
> local keymaps given that it is near certain to be a prefix of other
> key bindings.
Fair enough.
Your patch should be a nice usability improvement especially for beginners.
Roman.
Philippe Altherr wrote on Thu, Dec 01, 2022 at 20:30:22 +0100: > Yep, that remains correct. Same for ERR_EXIT. > > Most(/All?) of the patches address cases where the implementation wasn't > following the specification. > Shouldn't docs of ERR_EXIT explicitly mention that «! true» does not cause an exit? > The patch about the always statement makes it behave like compound > statements. Since the always statement isn't part of POSIX, this could be > seen either as a bug fix or a spec change (but rather of the always > statement than of ERR_EXIT). > > Philippe > > > On Thu, Dec 1, 2022 at 5:30 PM Bart Schaefer <schaefer@brasslantern.com> > wrote: > > > On Thu, Dec 1, 2022 at 6:42 AM Philippe Altherr > > <philippe.altherr@gmail.com> wrote: > > > > > > https://zsh.sourceforge.io/Doc/Release/Options.html#index-ERRRETURN. > > > > Will that doc remain correct with respect to what it says about > > ERR_EXIT after recent patches are considered? > > > > (Still on my TODO list, have been busier than I expected so far this week.) > >
[-- Attachment #1: Type: text/plain, Size: 668 bytes --] > > Shouldn't docs of ERR_EXIT explicitly mention that «! true» does not > cause an exit? Arguably there is much more that is missing. Zsh's ERR_EXIT <https://zsh.sourceforge.io/Doc/Release/Options.html#index-ERREXIT> specification <https://zsh.sourceforge.io/Doc/Release/Options.html#index-ERREXIT> doesn't even state that ERR_EXIT is disabled in the condition of if/while/etc. Zsh intends to be compatible with POSIX' "set -e" specification <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#set>. That specification is quite a bit longer. Should Zsh's specification repeat all of that? Or should it link to it? Philippe [-- Attachment #2: Type: text/html, Size: 974 bytes --]
On Sat, Dec 10, 2022 at 6:12 AM Philippe Altherr <philippe.altherr@gmail.com> wrote: > > POSIX' "set -e" specification. That specification is quite a bit longer. Should Zsh's specification repeat all of that? Or should it link to it? The zsh manual is not rigorously enough written to be treated as a "specification" and would become even less readable than Ray already accuses it of being if we tried. I'd prefer to give a summary and then reference the POSIX spec for details (but not via URL-link to it; URLs tend to get stale). Aside, Martijn Dekker raised a related question back in workers/43874 -- according to the POSIX chapter on Utilities (https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_01) there are a bunch of circumstances where a non-interactive shell is required to exit "as if the exit builtin were called". With ERR_EXIT, interactive shells are also required to exit in those circumstances. I'm pretty sure, though haven't re-tested, that zsh does not in fact exit in some of those (such as a redirection error with a special builtin) and in some of the cases where it does exit, it does not run the SIGEXIT trap as implied by "as if the exit builtin" (that being the actual reason for Martijn's message). And yes, I used a URL that might become stale, in this message. Nobody is ever going to need or to be able to go through the archives and fix that, whereas documentation would be expected to be kept up to date, a burden I don't wish to impose.
[-- Attachment #1: Type: text/plain, Size: 2316 bytes --] > > The zsh manual is not rigorously enough written to be treated as a > "specification" and would become even less readable than Ray already > accuses it of being if we tried. I'd prefer to give a summary and > then reference the POSIX spec for details (but not via URL-link to it; > URLs tend to get stale). My proposed doc update covers most points. So much so that I didn't even feel the need to link to the POSIX doc. So no URL-link problem :-) Shouldn't docs of ERR_EXIT explicitly mention that «! true» does not > cause an exit? This is implicitly covered by my proposed update. Philippe On Sun, Dec 11, 2022 at 2:24 AM Bart Schaefer <schaefer@brasslantern.com> wrote: > On Sat, Dec 10, 2022 at 6:12 AM Philippe Altherr > <philippe.altherr@gmail.com> wrote: > > > > POSIX' "set -e" specification. That specification is quite a bit longer. > Should Zsh's specification repeat all of that? Or should it link to it? > > The zsh manual is not rigorously enough written to be treated as a > "specification" and would become even less readable than Ray already > accuses it of being if we tried. I'd prefer to give a summary and > then reference the POSIX spec for details (but not via URL-link to it; > URLs tend to get stale). > > Aside, Martijn Dekker raised a related question back in workers/43874 > -- according to the POSIX chapter on Utilities > ( > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_08_01 > ) > there are a bunch of circumstances where a non-interactive shell is > required to exit "as if the exit builtin were called". With ERR_EXIT, > interactive shells are also required to exit in those circumstances. > I'm pretty sure, though haven't re-tested, that zsh does not in fact > exit in some of those (such as a redirection error with a special > builtin) and in some of the cases where it does exit, it does not run > the SIGEXIT trap as implied by "as if the exit builtin" (that being > the actual reason for Martijn's message). > > And yes, I used a URL that might become stale, in this message. > Nobody is ever going to need or to be able to go through the archives > and fix that, whereas documentation would be expected to be kept up to > date, a burden I don't wish to impose. > [-- Attachment #2: Type: text/html, Size: 3317 bytes --]
Below is an update to the CSI key sequence patch I posted in 51160. It adds a couple of test cases. There's also a new test case for the related code that ensures it loops for vi widgets that need to wait for an operator. After thinking about it, I changed the condition when it finds a complete CSI sequence. It will now keep a key binding for the first part of the CSI sequence if that binding extends into the parameters. There may be some use for this I haven't forseen and there isn't much gained in preventing it. A binding for the CSI prefix (escape or escape bracket) is still dropped in favour of undefined-key and consuming the full CSI sequence from the input buffer. I also added a comment to that condition. Oliver diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c index d90838f03..48691e8d0 100644 --- a/Src/Zle/zle_keymap.c +++ b/Src/Zle/zle_keymap.c @@ -1586,7 +1586,7 @@ getkeymapcmd(Keymap km, Thingy *funcp, char **strp) Thingy func = t_undefinedkey; char *str = NULL; int lastlen = 0, lastc = lastchar; - int timeout = 0; + int timeout = 0, csi = 0, startcsi; keybuflen = 0; keybuf[0] = 0; @@ -1636,7 +1636,30 @@ getkeymapcmd(Keymap km, Thingy *funcp, char **strp) } #endif } - if (!ispfx) + + /* CSI key sequences have a well defined structure so if we currently + * have an incomplete one, loop so the rest of it will be included in + * the key sequence if that arrives within the timeout. */ + if (keybuflen >= 3 && !csi) { + startcsi = keybuflen - 3; + csi = keybuf[startcsi] == '\033' && keybuf[keybuflen - 2] == '['; + } + if (csi) { + csi = keybuf[keybuflen - 2] != Meta && keybuf[keybuflen - 1] >= 0x20 + && keybuf[keybuflen - 1] <= 0x3f; + /* If we reach the end of a valid CSI sequence and the matched key + * binding is for part of the CSI introduction, select instead the + * undefined-key widget and consume the full sequence from the + * input buffer. */ + if (!csi && keybuf[keybuflen - 1] >= 0x40 && + keybuf[keybuflen - 1] <= 0x7e && lastlen > startcsi && + lastlen <= startcsi + 2) { + func = t_undefinedkey; + lastlen = keybuflen; + } + } + + if (!ispfx && !csi) break; } if(!lastlen && keybuflen) diff --git a/Test/X02zlevi.ztst b/Test/X02zlevi.ztst index 203c13c32..ccfb7b1c6 100644 --- a/Test/X02zlevi.ztst +++ b/Test/X02zlevi.ztst @@ -596,6 +596,13 @@ >BUFFER: 1ls `2` $(3) "4" $'5' ${6} >CURSOR: 0 + zpty_run 'bindkey -s -a "cw" "dwi"' + zletest $'one two\e0cwyksi' + zpty_run 'bindkey -r -a "cw"' +0:for a vi command, wait to allow a longer binding to be used +>BUFFER: yksitwo +>CURSOR: 4 + %clean zmodload -ui zsh/zpty diff --git a/Test/X03zlebindkey.ztst b/Test/X03zlebindkey.ztst index 5277332a7..1b63b3920 100644 --- a/Test/X03zlebindkey.ztst +++ b/Test/X03zlebindkey.ztst @@ -37,6 +37,28 @@ >"^Xy" "bar" >"^Xy" undefined-key + zpty_run 'bindkey -s "\e[" altbracket' + zletest $'$\C-A\e[17~' + zpty_run 'bindkey -r "\e["' +0:binding to CSI introduction is not used if a full sequence arrives +>BUFFER: $ +>CURSOR: 0 + + zpty_run 'bindkey -s "\e[1" altbracketone' + zletest $'$\C-A\e[17~' + zpty_run 'bindkey -r "\e[1"' +0:binding to longer prefix of a CSI sequence is used +# we assume the user knows what they're doing +>BUFFER: altbracketone7~$ +>CURSOR: 15 + + zpty_run 'bindkey -s "\e[" altbracket' + zletest $'$\C-A\e[177' + zpty_run 'bindkey -r "\e["' +0:use prefix binding where we don't have a CSI sequence +>BUFFER: altbracket177$ +>CURSOR: 13 + # As we're only looking at definitions here, we don't # bother using the pseudo-terminal; just test in the normal fashion. bindkey -e