Merge lp:~mterry/geonames/search-countries-too into lp:geonames

Proposed by Michael Terry
Status: Needs review
Proposed branch: lp:~mterry/geonames/search-countries-too
Merge into: lp:geonames
Diff against target: 73 lines (+34/-9)
2 files modified
src/geonames-query.c (+24/-9)
tests/test-geonames.c (+10/-0)
To merge this branch: bzr merge lp:~mterry/geonames/search-countries-too
Reviewer Review Type Date Requested Status
Geonames developers Pending
Review via email: mp+303864@code.launchpad.net

Commit message

Allow searching for a string that includes the country name, like "London, United Kingdom".

Description of the change

This is a quick and dirty fix, just to match previous behavior of Ubuntu System Settings before they switched to geonames (see linked bug).

Our search was never robust (only matched exact city prefixes). Ideally we'd do something really clever when searching.

But I didn't want to take on a whole search engine project, so I instead just tacked the country name onto the city name with a comma and matched against that.

Searches like "englishname, translatedname" or "translatedname, englishname" won't match correctly. For the same reason above, I didn't want to complicate our searching too much with combinatorial explosions of possibilities next time we tweak the algorithm.

To post a comment you must log in.

Unmerged revisions

27. By Michael Terry

Allow searching against country too

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/geonames-query.c'
2--- src/geonames-query.c 2016-04-20 15:49:48 +0000
3+++ src/geonames-query.c 2016-08-24 20:30:56 +0000
4@@ -148,17 +148,32 @@
5 {
6 const gchar *id;
7 const gchar *en_name;
8- const gchar *translation;
9+ const gchar *country_id;
10+ const gchar *country_en_name;
11+ const gchar *translated_name;
12+ const gchar *translated_country_name;
13 guint population;
14 gdouble best_weight = 0;
15-
16- g_variant_get_child (db, i, "(&s&s&s&s&s&s&sudd)", &id, &en_name, NULL, NULL, NULL, NULL, NULL, &population, NULL, NULL);
17-
18- best_weight = calculate_weight (query_tokens, en_name, population, best_weight);
19-
20- translation = g_dgettext (PACKAGE, id);
21- if (g_strcmp0 (translation, id) != 0)
22- best_weight = calculate_weight (query_tokens, translation, population, best_weight);
23+ g_autofree gchar *combined_en_name = NULL;
24+
25+ g_variant_get_child (db, i, "(&s&s&s&s&s&s&sudd)", &id, &en_name, NULL, NULL, &country_id, &country_en_name, NULL, &population, NULL, NULL);
26+
27+ // FIXME: We could benefit from a much more generic matching algorithm,
28+ // rather than only checking if we match the beginning substring. And
29+ // no matching against admin1 zone. And not smartly handling if country
30+ // name is given translated but city name isn't. Not removing
31+ // useless punctuation. etc
32+ combined_en_name = g_strdup_printf ("%s, %s", en_name, country_en_name);
33+ best_weight = calculate_weight (query_tokens, combined_en_name, population, best_weight);
34+
35+ translated_name = g_dgettext (PACKAGE, id);
36+ translated_country_name = g_dgettext (PACKAGE, country_id);
37+ if (g_strcmp0 (translated_name, id) != 0 || g_strcmp0 (translated_country_name, country_id) != 0)
38+ {
39+ g_autofree gchar *combined_translated_name = NULL;
40+ combined_translated_name = g_strdup_printf ("%s, %s", translated_name, translated_country_name);
41+ best_weight = calculate_weight (query_tokens, combined_translated_name, population, best_weight);
42+ }
43
44 if (best_weight > 0.0)
45 g_sequence_insert_sorted (matches, match_new (i, best_weight), compare_matches, NULL);
46
47=== modified file 'tests/test-geonames.c'
48--- tests/test-geonames.c 2016-04-04 18:21:31 +0000
49+++ tests/test-geonames.c 2016-08-24 20:30:56 +0000
50@@ -115,6 +115,15 @@
51 }
52
53 static void
54+test_full_query (void)
55+{
56+ assert_first_stats ("montreal, canada", "Montreal", "CA", 45.50884, -73.58781, 1600000);
57+
58+ change_lang ("fr_CA");
59+ assert_first_stats ("montréal, canada", "Montréal", "CA", 45.50884, -73.58781, 1600000);
60+}
61+
62+static void
63 test_edge_cases (void)
64 {
65 gint *indices;
66@@ -141,6 +150,7 @@
67
68 g_test_add_func ("/common-cities", test_common_cities);
69 g_test_add_func ("/translations", test_translations);
70+ g_test_add_func ("/full-query", test_full_query);
71 g_test_add_func ("/edge-cases", test_edge_cases);
72
73 return g_test_run ();

Subscribers

People subscribed via source and target branches