Closed dpecos closed 5 years ago
Could you please add tests like the other elements have?
Tests added :D
Just FYI, help interaction like for normal input is not implemented. I've added a test to actually check that.
Looks good to me and, as far as I can see, works as expected.
@AlecAivazis In case you are ok with merging it, we should also edit the Readme.
Hey @dpecos - thanks for adding those tests! Also thanks to @MarkusFreitag for watching over this.
I haven't yet tested this on a windows terminal (I see an ascii escape sequence in the source so my hunch is that it needs to be tweaked a little) so I am going to test it out there before giving it a 👍 and merging it.
Until I verify it (will hopefully get to tonight or tomorrow) could you add something to the Readme @dpecos that outlines its behavior? Don't need to worry about the screen recording, I can take care of that (unless you feel up for it).
So I finally got a chance to pull this down a down and give it a try. A few comments:
Empty line to finish
but i had to leave 2 empty lines. I understand this is to allow a responses with new lines between but it was sort of misleading. Maybe we can change that to Enter 2 empty lines to finish
In general, this prompt is a welcome addition to the family. Definitely excited to see this merged since I think it is unique from the rest 💯
Also, once this goes in, i'll start redoing the recordings (which has to happen anyway for #130)
I also played around with it a bit and actually like the 2 empty lines to finish
thingy. So we should keep it and update the help message.
Regarding the navigation: I would love to see this implemented, but I dont actually know how much effort this takes.
Agree, the message can be misleading, I've changed it to Enter 2 empty lines to finish
.
About using the cursors to navigate, it would be really nice, but the implementation for that is not trivial at all (at least right now I can't think about an easy way to get that done).
I would suggest keeping current behavior as it is and improve it in a future PR. I would be awesome to have that in my project. BTW, if you want to have a look into it: https://github.com/dpecos/cbox, still a WIP, but planning to release it in a couple of weeks (I still have to work on proper docs and an small tutorial).
Thank you for clarifying the instructional text. I agree that adding the ability to navigate the prompt can be done as a layer on top. I'm going to open an issue when this goes in and if you feel inspired to take it on and want someone to bounce ideas off of - please get in touch (you can find me on the gophers slack channel if you want something more direct than github issues).
I was thinking about this feature and I think the use of the word Input
is a little vague. Technically, all Prompts
could be named ...Input
so I've tried avoiding it. To be honest, every time I see the Input
prompt, I get a little annoyed with my self for picking something so generic. How would you feel if we renamed this to something shorter like Multiline
?
Your project looks very interesting! I'll definitely be keeping at eye on it as I have wished for something very similar on many occasions.
I spent some time tonight updating all of the screen recordings and got one for this input: https://thumbs.gfycat.com/DisgustingBonyAztecant-size_restricted.gif
After watching this recording a few times, I have the feeling that the response template should put the answer on a brand new line instead of on the same line as the original prompt. As it is now, it's kind of hard to read imo. What do you think?
For comparison, here is a recording with the line break present before the response: https://thumbs.gfycat.com/ImperfectShimmeringBeagle-size_restricted.gif
Thanks again for the contribution!
I was precisely thinking about this last night :)
I've done the change and adjusted the tests and also renamed the input to just Multiline
. I really don't mind renaming it.
About cursor navigation, I will try to implement it once I get some time available from cbox, because it's something that will make the app way nicer :)
Awesome! That would be great - I think this is good to go once you update the README to the recording I posted above. It's a little slow for some reason (doesn't look like that in the preview online) but it's better than having something that's not the right prompt.
Done!
About cursor navigation, I will try to implement it once I get some time available from cbox, because it's something that will make the app way nicer :)
Has this been implemented?
Basic multiline input. It keeps appending lines to the value until a double empty line is typed.