Skip to content
This repository has been archived by the owner on Jun 4, 2022. It is now read-only.

Add support for reverse-i-search #169

Merged
merged 5 commits into from
Jun 1, 2017
Merged

Add support for reverse-i-search #169

merged 5 commits into from
Jun 1, 2017

Conversation

anmonteiro
Copy link
Owner

No description provided.

session.reverseSearchBuffer = '';

if (clear) {
session.rl.write(null, {

Choose a reason for hiding this comment

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

Wouldn't readline.clearLine from the api also work?
https://nodejs.org/api/readline.html#readline_readline_clearline_stream_dir

Copy link
Owner Author

Choose a reason for hiding this comment

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

I did try that and it didn't seem to work for some reason.

@pesterhazy
Copy link

I love it! Comparing with rlwrap, it works almost identically (especially love that ^G works as expected). Assuming that the goal is to imitate GNU readline/rlwrap:

  • When rlwrap runs out of completions, it shows (failed reverse-i-search)alpha': alpha`
  • Cursor keys (left and right) should cancel reverse-i-search mode. ^F and ^B already work as expected

I had one inline comment, but otherwise this looks good to me. Actually it occurred to me this might be a great general-purpose node library or enhancement to https://www.npmjs.com/package/historic-readline (not saying you should put in the additional work of course).

@arichiardi
Copy link
Collaborator

arichiardi commented May 30, 2017

I was thinking exactly that when talking about upgradable general purpose node repl. Antonio, you are really raising the bar for node repls here 😉

@anmonteiro
Copy link
Owner Author

Thanks for your feedback! I'll definitely incorporate those features. I tried to follow rlwrap style reverse-i-search as close as possible but apparently missed some small behaviors :-)

@anmonteiro
Copy link
Owner Author

@pesterhazy just added a commit that addressed your early feedback. Mind giving it another go to see if it satisfies all requirements now?

@pesterhazy
Copy link

@anmonteiro just compiled the updated version, works fabulously. It's hard to tell the difference to "real" readline. 🥇

@anmonteiro anmonteiro force-pushed the reverse-search branch 2 times, most recently from c177ce2 to 199e62c Compare June 1, 2017 00:22
@anmonteiro
Copy link
Owner Author

@pesterhazy thanks so much for trying it out. Just did some cleanup wrt. duplicates & entering multiline forms.

Planning to ship it tomorrow morning if nothing major comes up.

Summary of the changes in this commit:

1. Set prompt to `(failed reverse-i-search)` when the reverse search fails
2. Have cursor keys cancel reverse search mode
@anmonteiro anmonteiro force-pushed the reverse-search branch 2 times, most recently from 574f4b2 to fee7301 Compare June 1, 2017 02:35
@anmonteiro anmonteiro merged commit 8486b14 into master Jun 1, 2017
@anmonteiro anmonteiro deleted the reverse-search branch June 1, 2017 16:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants