JacobEvelyn / friends

Spend time with the people you care about. Introvert-tested. Extrovert-approved.
MIT License
872 stars 39 forks source link

undefined method error occurred when “add tag" or “add nickname” executed with no arguments #265

Closed m-t-a-n-a-k-a closed 4 years ago

m-t-a-n-a-k-a commented 4 years ago

VERSION 
0.52(ruby 2.6.0)

SUMMARY 
When “add tag" or “add nickname” executed with no arguments, undefined method error messages are output as below.

$ friends add activity
2020-06-22: test
Activity added: "2020-06-22: test"
$ friends add note
2020-06-22: test
Note added: "2020-06-22: test"
$ friends add friend
Error: Expected "[Friend Name]"
$ friends add tag
Error: undefined method `strip' for nil:NilClass ⬅︎ ★
$ friends add location
Error: Expected "[Location Name]"
$ friends add nickname
Error: undefined method `strip' for nil:NilClass ⬅︎ ★

I think it would be better to change the error message as below.

$ friends add tag
Error: Expected "[Friend Name] [Tag]”
$ friends add nickname
Error: Expected "[Friend Name] [Nickname]”

Could you please let me know what you think about this change? Thank you.

JacobEvelyn commented 4 years ago

Good catch, @m-t-a-n-a-k-a! I agree with one small question for you. add tag is able to infer that only the last argument is the tag and everything else is a name, so these will both work:

$ friends add tag Jacob Evelyn @human
$ friends add tag "Jacob Evelyn" @human

but add nickname can't do that because nicknames can be multiple words along with friend names, so when either is multiple words you need to use quotes:

$ friends add nickname Jacob Jake
$ friends add nickname "Jacob Evelyn" Jake
$ friends add nickname Jacob "Jake Evelyn"

Given that, do you think we should keep the add nickname message as you had it:

Error: Expected "[Friend Name] [Nickname]"

Or add quotes so that people don't get tripped up there:

Error: Expected "[Friend Name]" "[Nickname]"

?

m-t-a-n-a-k-a commented 4 years ago

Thank you for your reply, @JacobEvelyn! I agree with your newer idea. Indeed, I think the newer one is easier to understand.

Error: Expected "[Friend Name]" "[Nickname]"

In that case, what do you think of add tag? It would be better the following one in terms of uniformity.

Error: Expected "[Friend Name]" “[Tag]”
JacobEvelyn commented 4 years ago

Good call! I think I'd be comfortable with either of these:

Error: Expected "[Friend Name]" "[Tag]"
Error: Expected "[Friend Name]" [Tag]

Do you want to work on this or would you prefer that I make the change?

m-t-a-n-a-k-a commented 4 years ago

@JacobEvelyn Yes, I’d like to work on this issue. It probably won't take much time to create a PR for this. Please give me a few days👍

JacobEvelyn commented 4 years ago

Sounds perfect, thank you! ❤️ As always, let me know if I can help.