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

Issues found after the Reader rewrite #41

Closed
mitnk opened this issue May 21, 2018 · 6 comments
Closed

Issues found after the Reader rewrite #41

mitnk opened this issue May 21, 2018 · 6 comments

Comments

@mitnk
Copy link

mitnk commented May 21, 2018

  1. Methods set_history_size() and history_size are missing

  2. Both reader.rs and interface.rs has a set_prompt; and the color escape sequences seems not work any more.

  3. issue Ctrl-C Overwrites Multiple Line Inputs and Hisotry Search #38 still not fixed

@murarth
Copy link
Owner

murarth commented May 21, 2018

  1. The history_size field was removed because it conflicted with the Reader/Writer split. It was a "variable," so it theoretically belongs in Reader, with the other user-settable variables, but the history is part of Writer because it's used in rendering the prompt to the screen. I suppose it could be restored without being a "variable." I'll see about that.
  2. There are a number of methods duplicated this way. They exist on the type that truly implements the functionality and they exist on Interface as a convenience. My expectation is that single-threaded programs will likely only use Interface directly and such convenience methods facilitate that.
    I did screw up the escape sequence thing, though. I'll have a fix for that.
  3. Issue Ctrl-C Overwrites Multiple Line Inputs and Hisotry Search #38 seems fixed to me. Have you pulled all of the new commits?

@murarth
Copy link
Owner

murarth commented May 21, 2018

Okay, the following fixes have been made:

  1. set_history_size and friends are back. The new methods are implemented on Interface. I realized while making this fix that removing history entries during a read_line call can be hazardous, so I've removed such methods from Prompter. This does mean that, say, a user-implemented Function trait can no longer remove history during line-editing. This shouldn't have been possible in the first place, but the original all-in-one Reader type paradigm was simply too permissive.
  2. mortal (the new low-level terminal implementation crate) has been fixed to leave escape sequences as-is in written text. Before the next linefeed release is published, I will publish a new version of mortal containing this fix and update linefeed's dependency version.
  3. I'm not sure if this is what you were referring to, but I've changed the echo-control-characters behavior of Ctrl-C so that ^C is written out at the end of the input instead of at the current cursor position. The old behavior was consistent with GNU Readline, but that's not a good enough reason to keep it in this case. Overwriting previous user input is not useful at all.

@mitnk
Copy link
Author

mitnk commented May 21, 2018

Hi @murarth, Thanks for the great work you've done!

Thanks for 1 & 2.

For issue 3, addition to the description in #38, let me also upload several images to demo it:

Bash - Before Ctrl-C
bash-before-c

Bash - After Ctrl-C
bash-after-c

With linefeed:

lf-before-c

lf-after-c

Exactly same key inputs in cases in images above for bash and linefeed.

@murarth
Copy link
Owner

murarth commented May 21, 2018

Ah, I see. I had overlooked that the bug was specifically affecting reverse search inputs. I will take a look at this shortly.

@murarth
Copy link
Owner

murarth commented May 21, 2018

@mitnk: I think I've now fully addressed the issues with the incremental search prompt. Thank you for your attentiveness on the matter.

@mitnk
Copy link
Author

mitnk commented May 22, 2018

It works great, thanks! I'm closing this issue since only escape sequences left. Look forward to your coming release.

@mitnk mitnk closed this as completed May 22, 2018
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

No branches or pull requests

2 participants