Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Cmd::Yield for complex custom bindings #342

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tailhook
Copy link
Contributor

@tailhook tailhook commented Mar 6, 2020

This partially replaces #293, i.e. some custom bindings can be implemented like this, although using Yield-style bindings is more complex so PR's are mostly unrelated.

The idea behind the Yield building is that we return from the readline function with a special error value which holds user's value (Arc<dyn Any>) along with current input. This may be used by user's REPL loop for many complex to implement but useful things like:

  1. Changing prompt
  2. Changing mode of operation
  3. Changing Vi / Emacs mode like I've put in the yield_binding example as part of this PR

What do you think?

@rwestphal
Copy link

What's the status of this PR?

I'm not a maintainer but these changes look good to me.

@tailhook maybe you could move the matches!-related changes and the new example into separate commits to facilitate the review process.

@tailhook
Copy link
Contributor Author

@tailhook maybe you could move the matches!-related changes and the new example into separate commits to facilitate the review process.

I can move them before, not after. The issue is that Cmd doesn't have PartialEq after this change, so you can only match on it.

But generally I've not got any agreement that this functionality would be accepted. So I don't want to waste time on rebasing it until that. (And this PR is terribly outdated)

Also, the biggest issue with Yield being used for complex commands is that "undo" would not work well (this is still fine for other use cases, where you treat input differently if submitted by a different key binding, or if you have an external editor on a keybinding, though).

@rwestphal
Copy link

But generally I've not got any agreement that this functionality would be accepted. So I don't want to waste time on rebasing it until that. (And this PR is terribly outdated)

Fair enough. In the meanwhile I'll create a private fork of rustyline since I really need Cmd::Yield to implement a Cisco IOS-like CLI in Rust.

I hope this PR gets the attention it deserves as having the ability to bind custom behavior to certain keystrokes is crucial for any readline implementation.

Copy link

@rwestphal rwestphal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gwenn gwenn mentioned this pull request Feb 16, 2021
@Keating950
Copy link

Is this still being considered? I'm very interested in this feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants