Feature: Added celsius support for weather #19
Conversation
| @@ -99,7 +99,7 @@ function messageHandler(event) { | |||
| handlerFunctions['getPokemon'](userAPI, event.threadID, event.senderID, message); | |||
| } else if ((/^@stock .+$/).test(message)) { | |||
| handlerFunctions['getStock'](userAPI, event.threadID, message); | |||
| } else if ((/^@weather ([0-9]{5}|([a-zA-Z ]+(, )?[a-zA-Z ]+))$/).test(message)) { | |||
| } else if ((/^@weather ([0-9]{5} [CF]|([a-zA-Z ]+(, )?[a-zA-Z ]+))$/).test(message)) { | |||
There was a problem hiding this comment.
Can you make the degree type into flags --unit <C/F> and -u <C/F>? And modify the regex accordingly to sanitize the input.
There was a problem hiding this comment.
Not quite sure how to do that? Any tutorials?
There was a problem hiding this comment.
or you want a format like @weather 78701 --unit C
There was a problem hiding this comment.
Yeah like that. @weather 78701 --unit C, or @weather 78701 -u F, or lower cases.
If we use flags, we are guaranteed that the characters after the - are not part of the address, so we can simply substring the flag out, and use the @weather 78701 part of the body the old way.
|
You can use this regex string to match the flags
|
|
|
||
| __Arguments__ | ||
|
|
||
| * `zipcode`: A five digit US zipcode. | ||
| * `location name`: A letter and space only location name, or city name and state separated by a comma and a space. | ||
| * `degree type`: A degree type to display the weather, the parameter can be 'C' for Celsius or 'F' for Fahrenheit. The default type is Fahrenheit. |
There was a problem hiding this comment.
I guess we can also support lower case for lazy people who don't want to press shift.
There was a problem hiding this comment.
Yeah I was thinking about that, I'm going to do some toUpperCase stuff and it should be good to go.
There was a problem hiding this comment.
I feel like flags are overkill? This is messenger, not bash.
There was a problem hiding this comment.
@rjchee I might stick with the original implementation with the updated regex so hopefully the nasty loop will disappear.
There was a problem hiding this comment.
@rjchee The reason behind the flag is to sanitize the input. Right now we are polling the last character of the address, which is pretty sketch. Also, if we designate the flag syntax, then we can reuse old code after substringing the flag out, so we don't need to do deep regression testing.
There was a problem hiding this comment.
@renxinhe Why can't we just poll for a space and a C or an F? I doubt there are locations that match that, and if there were, users can just tack on the degree type they want to use. Then the old code can still be reused, and we don't have to introduce bash to non-programmer users.
|
And when you are done, can you just bump the version in |
| @@ -172,12 +172,13 @@ This posts weather information of a given location. | |||
|
|
|||
| __Usage__ | |||
|
|
|||
| `@weather <zipcode | location name>` | |||
| `@weather <zipcode | location name> > <degree type>` | |||
There was a problem hiding this comment.
Format this doc to adopt the flag notation.
No description provided.