Global input parse - Githubissues

iridakos / stup

Daily notes in the terminal :penguin:

https://github.com/iridakos/stup

MIT License

415 stars 31 forks source link

Global input parse #33

Closed ghost closed 4 years ago

ghost commented 4 years ago

This moves input parsing (yes|no, trim) into functions so we don't have to make one change on many places

ghost commented 4 years ago

What I was wondering about:

do we really need to echo -e user input? (see how it now looks like when you trim something)
we can use https://stackoverflow.com/a/3352015 or https://stackoverflow.com/a/369795 for trimming newlines (and it's w/o external tools), but then we would need to use sed anyway if we want to strip also whitespace at the beginning/end of an "inner" line. What's the "correct" solution for you?
do you want >>> at the beginning of every prompt? I removed most of them while changing to resolve_boolean

iridakos commented 4 years ago

:rocket: Thank you!!! :rocket:

I will check it and merge it a.s.a.p.

iridakos commented 4 years ago

First of all: thank you very much for your contributions!

do we really need to echo -e user input? (see how it now looks like when you trim something)

:+1: we don't need that

we can use https://stackoverflow.com/a/3352015 or https://stackoverflow.com/a/369795 for trimming newlines (and it's w/o external tools), but then we would need to use sed anyway if we want to strip also whitespace at the beginning/end of an "inner" line. What's the "correct" solution for you?

Pick your favorite.

do you want >>> at the beginning of every prompt? I removed most of them while changing to resolve_boolean

Nope, you can either remove the rest or else I can do it too.

ghost commented 4 years ago

I realised that the second StackOverflow answer did exactly what the very first solution (removed all whitespace), and the first one doesn't allow changing the #! to zsh, so I used https://unix.stackexchange.com/a/476504 instead.

Also, please keep this until #37 is merged, I have some changes for the ynaq there.

iridakos commented 4 years ago

Sure! Thank you :)

iridakos commented 4 years ago

@tiosgz Let me know whenever this PR is ready in order to merge it. #37 is already merged, you should be good to go.

ghost commented 4 years ago

I think it's ready. I can remove the TODO and add resolve_ynqa right now if you want.

iridakos commented 4 years ago

@tiosgz I agree with your comment. We can put it in another function once we see that there's a need to re-use it.

Thank you very much for your excellent contributions :pray: