Added OptionBreakLineCallback, to run a callback every time there's a line break #140
Conversation
… line break It's useful to run a function everytime there's a line break -- Enter as well as, for example, ControlC. With this PR, a new option is added to assign a callback that gets called every time `renderer.BreakLine()` is called. Added a test that makes sure the renderer doesn't break if the callback is not specified, as well as to check that it runs ok when the callback executes. Just to give a bit more of context: in [ABS](https://github.com/abs-lang/abs) we are trying to implement ControlR (reverse search), and need to clear the search selection every time the user "clears" the console, either by pressing enter or by clearing the current line (eg. ControlC).
|
Now I consider following interface (Add BreakLineCallback func(Document) |
render.go
Outdated
| @@ -12,6 +12,7 @@ type Render struct { | |||
| out ConsoleWriter | |||
| prefix string | |||
| livePrefixCallback func() (prefix string, useLivePrefix bool) | |||
| BreakLineCallback func() | |||
There was a problem hiding this comment.
@odino Could you add an argument of prompt.Document like completer? (please tell me if you have something your opinion about my proposal.)
There was a problem hiding this comment.
Done, let me know if you'd like anything else!
As general feedback: one thing that's currently hard with go-prompt is that most members are private, so accessing them is out of the question. There could be things like changing color after the user presses a specific keybind that could be very useful, but are now impossible to do since you can't access those members. My suggestion would be to review the public interface and maybe allow users to be able to access renderer, input parser, etc etc.
There was a problem hiding this comment.
Thank you for your feedbacks.
We can make the field public without breaking API changes. But we cannot make the public field private without breaking API changes. So in my opinion, I define the field as private if it's not necessary to make public. But as you said, we might need to review the public interface. Thanks!
There was a problem hiding this comment.
@odino Could you make BreaklineCallback private If there is no advantages to make it public in your usecases?
There was a problem hiding this comment.
Done, let me know if you need anything else!
There was a problem hiding this comment.
Thank you! LGTM!!
I'll merge this after passes CI checks!
|
@c-bata let me know if anything else is needed! |
|
@c-bata can we get this merged? 😄 |
|
I'm so sorry for late reply. Could you check this comment? |
|
Thank you for your contribution 👍 |
|
Welcome, and thanks for this amazing library :)
…On Mon, Aug 26, 2019 at 5:48 PM Masashi SHIBATA ***@***.***> wrote:
Thank you for your contribution 👍
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#140?email_source=notifications&email_token=AACQFZEMLHQ7FBRIU7RUE6TQGPNLFA5CNFSM4IMC4VYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5ENQGA#issuecomment-524867608>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACQFZBNVS7FJLFC6TPFKGDQGPNLFANCNFSM4IMC4VYA>
.
|
It's sometimes useful to run a function everytime there's a line break -- Enter as well as, for example, ControlC.
With this PR, a new option is added to assign a callback that gets
called every time
renderer.BreakLine()is called.Added a test that makes sure the renderer doesn't break if the callback is
not specified, as well as to check that it runs ok when the callback executes.
Just to give a bit more of context: in ABS
we are trying to implement ControlR (reverse search), and need to clear
the search selection every time the user "clears" the console, either by
pressing enter or by clearing the current line (eg. ControlC).