PATCH: Add SHORT_KEYBIND_DISCARD option

PATCH: Add SHORT_KEYBIND_DISCARD option

[Mikael Magnusson]

Not sure how popular this option would be, but I'm fine with keeping
it local too. Basically when we encounter an undefined key, instead
of discarding the entire common prefix, we discard only one byte and
try again. In most particular, it means if you happen to press escape
or ctrl-x before pasting, we still properly detect the bracketed-paste
starting code.

---
 Src/Zle/zle_keymap.c | 8 ++++++--
 Src/options.c        | 1 +
 Src/zsh.h            | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Src/Zle/zle_keymap.c b/Src/Zle/zle_keymap.c
index 7b11933..e8de0cd 100644
--- a/Src/Zle/zle_keymap.c
+++ b/Src/Zle/zle_keymap.c
@@ -1610,8 +1610,12 @@ getkeymapcmd(Keymap km, Thingy *funcp, char **strp)
 	if (!ispfx)
 	    break;
     }
-    if(!lastlen && keybuflen)
-	lastlen = keybuflen;
+    if(!lastlen && keybuflen) {
+	if (isset(SHORTKEYBINDDISCARD))
+	    lastlen = 1;
+	else
+	    lastlen = keybuflen;
+    }
     else
 	lastchar = lastc;
     if(lastlen != keybuflen) {
diff --git a/Src/options.c b/Src/options.c
index 008ebf1..fa20eb9 100644
--- a/Src/options.c
+++ b/Src/options.c
@@ -248,6 +248,7 @@ static struct optname optns[] = {
 {{NULL, "shinstdin",	      OPT_SPECIAL},		 SHINSTDIN},
 {{NULL, "shnullcmd",          OPT_EMULATE|OPT_BOURNE},	 SHNULLCMD},
 {{NULL, "shoptionletters",    OPT_EMULATE|OPT_BOURNE},	 SHOPTIONLETTERS},
+{{NULL, "shortkeybinddiscard",0},                        SHORTKEYBINDDISCARD},
 {{NULL, "shortloops",	      OPT_EMULATE|OPT_NONBOURNE},SHORTLOOPS},
 {{NULL, "shwordsplit",	      OPT_EMULATE|OPT_BOURNE},	 SHWORDSPLIT},
 {{NULL, "singlecommand",      OPT_SPECIAL},		 SINGLECOMMAND},
diff --git a/Src/zsh.h b/Src/zsh.h
index cbaa351..0dc3248 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -2358,6 +2358,7 @@ enum {
     SHNULLCMD,
     SHOPTIONLETTERS,
     SHORTLOOPS,
+    SHORTKEYBINDDISCARD,
     SHWORDSPLIT,
     SINGLECOMMAND,
     SINGLELINEZLE,
-- 
2.6.1

Re: PATCH: Add SHORT_KEYBIND_DISCARD option

[Oliver Kiddle]

On 3 Apr, Mikael Magnusson wrote:
> Not sure how popular this option would be, but I'm fine with keeping
> it local too. Basically when we encounter an undefined key, instead
> of discarding the entire common prefix, we discard only one byte and
> try again. In most particular, it means if you happen to press escape
> or ctrl-x before pasting, we still properly detect the bracketed-paste
> starting code.

As a local fix, wouldn't simply binding the start code with each prefix
serve your purposes?
Didn't we discuss this issue a few months ago - have you perhaps got a
message reference? I haven't had the impression that it is bothering a
lot of people. And even without bracketed-paste, a prefix followed by a
paste is likely to have had undesirable results.

ZLE-specific stuff should ideally not leak into the base shell options.
Or we're you actually hoping to stimulate thoughts on other solutions:
perhaps a callback could be added like zle-keymap-select so you could
change the cursor shape or prompt after a prefix.

Oliver

Re: PATCH: Add SHORT_KEYBIND_DISCARD option

[Mikael Magnusson]

On Wed, Apr 6, 2016 at 4:33 PM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> On 3 Apr, Mikael Magnusson wrote:
>> Not sure how popular this option would be, but I'm fine with keeping
>> it local too. Basically when we encounter an undefined key, instead
>> of discarding the entire common prefix, we discard only one byte and
>> try again. In most particular, it means if you happen to press escape
>> or ctrl-x before pasting, we still properly detect the bracketed-paste
>> starting code.
>
> As a local fix, wouldn't simply binding the start code with each prefix
> serve your purposes?

For just making bracketed-paste work, yeah.

> Didn't we discuss this issue a few months ago - have you perhaps got a
> message reference? I haven't had the impression that it is bothering a
> lot of people. And even without bracketed-paste, a prefix followed by a
> paste is likely to have had undesirable results.

I think I asked if doing what this patch does was possible, but I want
it in general, not only because of bracketed-paste. If it were up to
me, this way would be the only way. It doesn't really make any sense
to me why we arbitrarily eat input just because it was a potential
common prefix that then turned out to not be part of any actual
keybinding. But since zsh people tend to like their things not
changing, I did the option.

I actually think it was so long ago I asked that the reason was we
didn't have bracketed-paste in core yet, and the weird multi-eating
meant you couldn't actually safely paste anything, because the evil
input could end in a potential prefix, and the bindkey -M paste stuff
wouldn't detect the end-paste code.

It just seems like a much more correct way of doing it to me.

> ZLE-specific stuff should ideally not leak into the base shell options.

Not sure what this is about, there are 113 calls to isset() in Src/Zle/*

> Or we're you actually hoping to stimulate thoughts on other solutions:
> perhaps a callback could be added like zle-keymap-select so you could
> change the cursor shape or prompt after a prefix.

Well, I want only one character to be eaten (or rather, only
characters that individually are "undefined-key"), rather than the
whole 'not actually a prefix'. That's my end goal. I guess I don't
currently have any super compelling arguments for it other than "it
seems much more correct".

Maybe I'm overestimating the amount of people depending on the current
behavior, and you would be more open to just changing the behavior
without adding any option? In the end, I can definitely live with the
patch being rejected though.

-- 
Mikael Magnusson

Re: PATCH: Add SHORT_KEYBIND_DISCARD option

[Bart Schaefer]

On Apr 6,  8:06pm, Mikael Magnusson wrote:
} Subject: Re: PATCH: Add SHORT_KEYBIND_DISCARD option
}
} > ZLE-specific stuff should ideally not leak into the base shell options.
} 
} Not sure what this is about, there are 113 calls to isset() in Src/Zle/*

ZLE used to be part of the main shell, so the options were already there
when it got peeled out into a module; but theoretically since then we are
trying (probably not always successfully) to avoid having any additional
co-dependency.  (The lexer/completion situation I mentioned in another
thread is a similar pre-module side-effect.)

} Well, I want only one character to be eaten (or rather, only
} characters that individually are "undefined-key"), rather than the
} whole 'not actually a prefix'.
} 
} Maybe I'm overestimating the amount of people depending on the current
} behavior, and you would be more open to just changing the behavior
} without adding any option?

I was going to suggest that, though I'm not sure eating one character is
really sufficient.  What if you get four characters of a five-character
prefix and then something you don't recognize?  To actually be emacs-like,
we'd have to go back and try the first three, and if still nothing the
first two, and finally replay everything you backed over and maybe do it
all again.

But ZLE keymaps/binding don't really layer that way.  I suppose most of
the time the prefix is only one character and things would work out.

Re: PATCH: Add SHORT_KEYBIND_DISCARD option

[Oliver Kiddle]

Mikael Magnusson wrote:
> > Didn't we discuss this issue a few months ago - have you perhaps got a
> > message reference?

I managed to find this again: users/20358 is a good reference where Bart
makes the point that this will have unpredictable effects for an unbound
function key. Some of the thread is in -workers.

We could make discarding the keys conditional upon whether KEYTIMEOUT
has elapsed since the prefix was received to account for that, however.

> I think I asked if doing what this patch does was possible, but I want
> it in general, not only because of bracketed-paste. If it were up to
> me, this way would be the only way. It doesn't really make any sense
> to me why we arbitrarily eat input just because it was a potential
> common prefix that then turned out to not be part of any actual
> keybinding. But since zsh people tend to like their things not
> changing, I did the option.

If get a prefixed key combination wrong, a beep seems cleaner than
getting the latter part inserted. 

> It just seems like a much more correct way of doing it to me.

But is it also proving an irritation in actual practice? Are you often
typing part of a long emacs-style binding and then being distracted by
something in another window, perhaps? Would a separate timeout after
which a prefix is discarded perhaps make sense?

Oliver

Re: PATCH: Add SHORT_KEYBIND_DISCARD option

[Mikael Magnusson]

On Thu, Apr 7, 2016 at 5:25 PM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> Mikael Magnusson wrote:
>> > Didn't we discuss this issue a few months ago - have you perhaps got a
>> > message reference?
>
> I managed to find this again: users/20358 is a good reference where Bart
> makes the point that this will have unpredictable effects for an unbound
> function key. Some of the thread is in -workers.

I'm not sure what exactly is unpredictable about it. Pressing F1,
you'll see [11~ instead of just the ~. (this is assuming you have some
other F-key bound, if you don't, you'll see 1~ because there's less
common prefix, so you're arguing for something that is already very
inconsistent). I could buy this argument if we ignored the whole key
including the final ~ too, but we don't. In emacs mode it gives you
more hints about what key you actually pressed, and in vi mode, ^[ is
already bound to vi-cmd-mode so the patch doesn't affect this case (it
already caseswaps 11 characters and leaves the user wondering wtf
happened).

> We could make discarding the keys conditional upon whether KEYTIMEOUT
> has elapsed since the prefix was received to account for that, however.
>
>> I think I asked if doing what this patch does was possible, but I want
>> it in general, not only because of bracketed-paste. If it were up to
>> me, this way would be the only way. It doesn't really make any sense
>> to me why we arbitrarily eat input just because it was a potential
>> common prefix that then turned out to not be part of any actual
>> keybinding. But since zsh people tend to like their things not
>> changing, I did the option.
>
> If get a prefixed key combination wrong, a beep seems cleaner than
> getting the latter part inserted.

People do things like bindkey -M viins jj vi-cmd-mode, but i'm sure
they still like to type words beginning with a j. (This works already
because j isn't undefined-key though).

You do have a point for things like ^X^D type stuff though, if I don't
have that bound maybe I don't want to accidentally close the shell.
(For weird people that don't use ignoreeof). :)

In my favor though, consider pressing alt-up from zsh -f in emacs
mode. Without the change, it will beep and insert [A, and with the
change it will beep (from the single escape), and then invoke the
up-line-or-history binding of the up key. That's better, arguably.

>> It just seems like a much more correct way of doing it to me.
>
> But is it also proving an irritation in actual practice? Are you often
> typing part of a long emacs-style binding and then being distracted by
> something in another window, perhaps?

I suppose it isn't.

> Would a separate timeout after which a prefix is discarded perhaps make sense?

I've sometimes harbored thoughts about allowing per-prefix timeouts even :).



Overall, I guess I'm a little bit less decided now than when I started
the thread.

-- 
Mikael Magnusson