archived-bot icon indicating copy to clipboard operation
archived-bot copied to clipboard

Weather command not parsing location names with spaces

Open SuhanKoh opened this issue 7 years ago • 10 comments

Fixing issue reported by user, #495

SuhanKoh avatar Jun 20 '18 03:06 SuhanKoh

The regex still won't work for town names containing characters that are not A-Z, for instance:

Tjæreborg
São Paulo
Москва
Clermond-Ferrand
東京

You might also need to trim() the string as well

freyacodes avatar Jun 21 '18 10:06 freyacodes

You might also want to target the sentinel branch, as this file has been migrated to Kotlin

freyacodes avatar Jun 21 '18 11:06 freyacodes

@Frederikam I feel like migration to kotlin for this change can be on a separate pr. I created this pr intended to fix the issue with spaces in the query

SuhanKoh avatar Jun 22 '18 02:06 SuhanKoh

I don't think openweather support searching 東京

SuhanKoh avatar Jun 22 '18 02:06 SuhanKoh

Perhaps not, but query parameters needs to be URL encoded. I don't know if the URL builder handles that

freyacodes avatar Jun 22 '18 05:06 freyacodes

@Frederikam It does, https://square.github.io/okhttp/3.x/okhttp/okhttp3/HttpUrl.Builder.html#addQueryParameter-java.lang.String-java.lang.String-

SuhanKoh avatar Jun 22 '18 05:06 SuhanKoh

Is it just me or does this look like a good case for a unit test?

schnapster avatar Jun 22 '18 15:06 schnapster

The API does not seem to support 東京 or Clermond-Ferrand, but the others work when using curl

freyacodes avatar Jul 06 '18 18:07 freyacodes

The latest commit supports the other ones except those two. Do you want me to add tests to it? or proceed with integrating with kotlin/sentinel branch?

SuhanKoh avatar Jul 06 '18 20:07 SuhanKoh

If I were you I would migrate to sentinel. I have developed an API to run integration tests on commands that you can use

freyacodes avatar Jul 06 '18 20:07 freyacodes