Make a copy of key_sequences in _call_handler

[asmeurer]

The _process method mutates the list of key sequences (calls del on it), which
ends up breaking the previous_key_sequences. Previously, it was just a list of
the current key sequence, but now it is correctly a list of the previously
handled key sequence.

Fixes #468.

[asmeurer]

Something I noticed when testing this. Key objects compare by identity, not by name, so you can't just do something like if event.previous_key_sequence == [KeyPress(key=Key('<Up>'), data='\x1b[A')] because the two Key objects compare differently. Is this intentional, or should Key("<Up>") == Key("<Up>") be True?

[asmeurer]

Also annoying that the keys can be strings or Key objects. Why not always convert them to a Key for the KeyPress.key attribute?

[asmeurer]

Similarly, why can't the default for previous_key_sequences be an empty list (instead of None)? I have to do a lot of annoying type checking just to implement this:

@r.add_binding(Keys.Enter, filter=is_returnable) def accept_after_history_backward(event): if pks and len(pks) == 1 and isinstance(pks[0].key, Key) and pks[0].key.name == "<Up>": accept_line(event) else: multiline_enter(event)

[jonathanslenders]

Good catch, thanks!

About comparing key objects. This needs to be super fast. This is really important in order to efficiently look up key handlers in a hash table. But what we can do, is caching all Key instances. Just memoize the __init__. I can do that.

Agreed about making previous_key_sequences an empty list. None should be avoided if possible.

Thanks!

[asmeurer]

Depending on memoization for equality testing means the cache never gets full, but I guess there are fewer than 1024 possible keys (right?).

[asmeurer]

Your @memoized decorator breaks isinstance on classes.

[asmeurer]

What about the Key vs. str change? Looking closer, I guess prompt_toolkit always treats printable keys as strings and meta characters as Key objects. It would be cleaner (IMO) if everything was converted to a Key internally.

[jonathanslenders]

The Key vs str change was a good point. It's a change that I committed to the 2.0 branch (for testing, but probably I'll keep it). I removed the Key class and everything becomes a string.

@memoized can be placed above __new__. Not above the class itself. (But for 2.0, we won't need that anymore.)

[jonathanslenders]

Fixed and merged in: 96a6435 + added unit tests.

Closing this. Feel free to open again if I missed something.