From 4ca9232933f7c7e74d25efb4064ee9e38d094cfb Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sat, 25 Apr 2020 23:00:19 +0200 Subject: [PATCH 1/8] test: Use g_assert_* instead of g_assert everywhere Theoretically, g_assert can be disabled. Also, g_assert_* print extra information. --- libgweather/test_libgweather.c | 40 +++++++++++++++++----------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/libgweather/test_libgweather.c b/libgweather/test_libgweather.c index f152a4fe..18038ee1 100644 --- a/libgweather/test_libgweather.c +++ b/libgweather/test_libgweather.c @@ -43,7 +43,7 @@ test_named_timezones (void) guint i; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); children = gweather_location_get_children (world); for (i = 0; children[i] != NULL; i++) { @@ -132,7 +132,7 @@ test_named_timezones_deserialized (void) GList *list, *l; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); list = get_list_from_configuration (world, CONFIGURATION, 3); for (l = list; l != NULL; l = l->next) @@ -171,7 +171,7 @@ test_no_code_serialize (void) g_assert_nonnull (world); loc = gweather_location_find_nearest_city (world, 56.833333, 53.183333); - g_assert (loc); + g_assert_nonnull (loc); g_assert_cmpstr (gweather_location_get_name (loc), ==, "Izhevsk"); g_assert_null (gweather_location_get_code (loc)); @@ -204,7 +204,7 @@ test_timezone (GWeatherLocation *location) GWeatherTimezone **tzs; tzs = gweather_location_get_timezones (location); - g_assert (tzs); + g_assert_nonnull (tzs); /* Only countries should have multiple timezones associated */ if ((tzs[0] == NULL && gweather_location_get_level (location) < GWEATHER_LOCATION_WEATHER_STATION) && @@ -247,7 +247,7 @@ test_timezones (void) GWeatherLocation *world; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); test_timezones_children (world); @@ -296,7 +296,7 @@ test_airport_distance_sanity (void) GWeatherLocation *world; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); test_airport_distance_children (world); @@ -418,12 +418,12 @@ test_metar_weather_stations (void) char *contents; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); msg = soup_message_new ("GET", METAR_SOURCES); session = soup_session_new (); soup_session_send_message (session, msg); - g_assert (SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)); + g_assert_true (SOUP_STATUS_IS_SUCCESSFUL (msg->status_code)); g_object_unref (session); g_assert_nonnull (msg->response_body); @@ -453,15 +453,15 @@ set_gsettings (void) g_assert_nonnull (tmpdir); /* Copy the schemas files */ - g_assert (g_file_get_contents (SCHEMAS_BUILDDIR "/org.gnome.GWeather.enums.xml", &schema_text, NULL, NULL)); + g_assert_true (g_file_get_contents (SCHEMAS_BUILDDIR "/org.gnome.GWeather.enums.xml", &schema_text, NULL, NULL)); dest = g_strdup_printf ("%s/org.gnome.GWeather.enums.xml", tmpdir); - g_assert (g_file_set_contents (dest, schema_text, -1, NULL)); + g_assert_true (g_file_set_contents (dest, schema_text, -1, NULL)); g_free (dest); g_free (schema_text); - g_assert (g_file_get_contents (SCHEMASDIR "/org.gnome.GWeather.gschema.xml", &schema_text, NULL, NULL)); + g_assert_true (g_file_get_contents (SCHEMASDIR "/org.gnome.GWeather.gschema.xml", &schema_text, NULL, NULL)); dest = g_strdup_printf ("%s/org.gnome.GWeather.gschema.xml", tmpdir); - g_assert (g_file_set_contents (dest, schema_text, -1, NULL)); + g_assert_true (g_file_set_contents (dest, schema_text, -1, NULL)); g_free (dest); g_free (schema_text); @@ -470,8 +470,8 @@ set_gsettings (void) "--schema-file=%s/org.gnome.GWeather.enums.xml " "--schema-file=%s/org.gnome.GWeather.gschema.xml", tmpdir, SCHEMAS_BUILDDIR, SCHEMASDIR); - g_assert (g_spawn_command_line_sync (cmdline, NULL, NULL, &result, NULL)); - g_assert (result == 0); + g_assert_true (g_spawn_command_line_sync (cmdline, NULL, NULL, &result, NULL)); + g_assert_cmpint (result, ==, 0); g_free (cmdline); /* Set envvar */ @@ -581,7 +581,7 @@ test_bad_duplicate_weather_stations (void) g_setenv ("LIBGWEATHER_LOCATIONS_NO_NEAREST", "1", TRUE); world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); stations_ht = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, (GDestroyNotify) NULL); @@ -643,7 +643,7 @@ test_duplicate_weather_stations (void) g_setenv ("LIBGWEATHER_LOCATIONS_NO_NEAREST", "1", TRUE); world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); test_duplicate_weather_stations_children (world); @@ -657,10 +657,10 @@ test_location_names (void) GWeatherLocation *world, *brussels; world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); brussels = gweather_location_find_nearest_city (world, 50.833333, 4.333333); - g_assert (brussels); + g_assert_nonnull (brussels); g_assert_cmpstr (gweather_location_get_name (brussels), ==, "Brussels"); g_assert_cmpstr (gweather_location_get_sort_name (brussels), ==, "brussels"); g_assert_cmpstr (gweather_location_get_english_name (brussels), ==, "Brussels"); @@ -670,10 +670,10 @@ test_location_names (void) _gweather_location_reset_world (); world = gweather_location_get_world (); - g_assert (world); + g_assert_nonnull (world); brussels = gweather_location_find_nearest_city (world, 50.833333, 4.333333); - g_assert (brussels); + g_assert_nonnull (brussels); g_assert_cmpstr (gweather_location_get_name (brussels), ==, "Bruxelles"); g_assert_cmpstr (gweather_location_get_sort_name (brussels), ==, "bruxelles"); g_assert_cmpstr (gweather_location_get_english_name (brussels), ==, "Brussels"); -- GitLab From 2ba0630d1aa310089d247da7d7185bc3b2afc8c9 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 27 Apr 2020 15:35:58 +0200 Subject: [PATCH 2/8] test: Fix small memory leak found by valgrind --- libgweather/test_libgweather.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libgweather/test_libgweather.c b/libgweather/test_libgweather.c index 18038ee1..fba947b3 100644 --- a/libgweather/test_libgweather.c +++ b/libgweather/test_libgweather.c @@ -448,8 +448,7 @@ set_gsettings (void) int result; /* Create the installed schemas directory */ - tmpdir = g_strdup_printf ("libgweather-test-XXXXXX"); - tmpdir = g_dir_make_tmp (tmpdir, NULL); + tmpdir = g_dir_make_tmp ("libgweather-test-XXXXXX", NULL); g_assert_nonnull (tmpdir); /* Copy the schemas files */ -- GitLab From 9572f910b3e15e7f17a2829ae388df08a61c74db Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Mon, 27 Apr 2020 16:04:41 +0200 Subject: [PATCH 3/8] test: Check that countries have one or more timezones It looks like the test should be inclusive for countries. Changing this actually makes the test able to catch more errors. --- libgweather/test_libgweather.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libgweather/test_libgweather.c b/libgweather/test_libgweather.c index fba947b3..f482fcbc 100644 --- a/libgweather/test_libgweather.c +++ b/libgweather/test_libgweather.c @@ -208,7 +208,7 @@ test_timezone (GWeatherLocation *location) /* Only countries should have multiple timezones associated */ if ((tzs[0] == NULL && gweather_location_get_level (location) < GWEATHER_LOCATION_WEATHER_STATION) && - gweather_location_get_level (location) > GWEATHER_LOCATION_COUNTRY) { + gweather_location_get_level (location) >= GWEATHER_LOCATION_COUNTRY) { g_print ("Location '%s' does not have an associated timezone\n", gweather_location_get_name (location)); g_test_fail (); -- GitLab From 97554113ebc9b74cf20249a4165da8636c131e18 Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sun, 26 Apr 2020 14:57:33 +0200 Subject: [PATCH 4/8] location-entry: Use internal API to create detached locations The location entry had a copy of the function that is already provided. Remove the copy. --- libgweather/gweather-location-entry.c | 54 ++------------------------- 1 file changed, 3 insertions(+), 51 deletions(-) diff --git a/libgweather/gweather-location-entry.c b/libgweather/gweather-location-entry.c index b6c7e236..6f5e7573 100644 --- a/libgweather/gweather-location-entry.c +++ b/libgweather/gweather-location-entry.c @@ -67,12 +67,6 @@ static void set_location_internal (GWeatherLocationEntry *entry, GtkTreeModel *model, GtkTreeIter *iter, GWeatherLocation *loc); -static GWeatherLocation * -create_new_detached_location (GWeatherLocation *nearest_station, - const char *name, - gboolean latlon_valid, - gdouble latitude, - gdouble longitude); static void fill_location_entry_model (GtkTreeStore *store, GWeatherLocation *loc, const char *parent_display_name, @@ -728,9 +722,9 @@ match_selected (GtkEntryCompletion *completion, loc = geocode_place_get_location (place); location = gweather_location_find_nearest_city (scope, geocode_location_get_latitude (loc), geocode_location_get_longitude (loc)); - location = create_new_detached_location(location, display_name, TRUE, - geocode_location_get_latitude (loc) * M_PI / 180.0, - geocode_location_get_longitude (loc) * M_PI / 180.0); + location = _gweather_location_new_detached (location, display_name, TRUE, + geocode_location_get_latitude (loc) * M_PI / 180.0, + geocode_location_get_longitude (loc) * M_PI / 180.0); set_location_internal (entry, model, NULL, location); @@ -849,45 +843,3 @@ gweather_location_entry_new (GWeatherLocation *top) "top", top, NULL); } - -static GWeatherLocation * -create_new_detached_location (GWeatherLocation *nearest_station, - const char *name, - gboolean latlon_valid, - gdouble latitude, - gdouble longitude) -{ - GWeatherLocation *self; - char *normalized; - - self = g_slice_new0 (GWeatherLocation); - self->ref_count = 1; - self->level = GWEATHER_LOCATION_DETACHED; - self->english_name = g_strdup (name); - self->local_name = g_strdup (name); - - normalized = g_utf8_normalize (name, -1, G_NORMALIZE_ALL); - self->english_sort_name = g_utf8_casefold (normalized, -1); - self->local_sort_name = g_strdup (self->english_sort_name); - g_free (normalized); - - self->parent = nearest_station; - self->children = NULL; - - if (nearest_station) - self->station_code = g_strdup (nearest_station->station_code); - - g_assert (nearest_station || latlon_valid); - - if (latlon_valid) { - self->latlon_valid = TRUE; - self->latitude = latitude; - self->longitude = longitude; - } else { - self->latlon_valid = nearest_station->latlon_valid; - self->latitude = nearest_station->latitude; - self->longitude = nearest_station->longitude; - } - - return self; -} -- GitLab From 34addd516694283527cc8fa4ba79b60d56f9680d Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sun, 26 Apr 2020 21:44:46 +0200 Subject: [PATCH 5/8] location-entry: Limit location searching to selected top node The top node may not be the a world node. So use the private top node instead and only try to do a country code based lookup if the top node is actually of level WORLD. --- libgweather/gweather-location-entry.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/libgweather/gweather-location-entry.c b/libgweather/gweather-location-entry.c index 6f5e7573..221a0f93 100644 --- a/libgweather/gweather-location-entry.c +++ b/libgweather/gweather-location-entry.c @@ -700,7 +700,11 @@ match_selected (GtkEntryCompletion *completion, GtkTreeIter *iter, gpointer entry) { - if (model != ((GWeatherLocationEntry *)entry)->priv->model) { + GWeatherLocationEntryPrivate *priv; + + priv = ((GWeatherLocationEntry *)entry)->priv; + + if (model != priv->model) { GeocodePlace *place; char *display_name; GeocodeLocation *loc; @@ -714,10 +718,10 @@ match_selected (GtkEntryCompletion *completion, -1); country_code = geocode_place_get_country_code (place); - if (country_code != NULL) - scope = gweather_location_find_by_country_code (gweather_location_get_world (), country_code); - else - scope = gweather_location_get_world (); + if (country_code != NULL && gweather_location_get_level (priv->top) == GWEATHER_LOCATION_WORLD) + scope = gweather_location_find_by_country_code (priv->top, country_code); + if (!scope) + scope = priv->top; loc = geocode_place_get_location (place); location = gweather_location_find_nearest_city (scope, geocode_location_get_latitude (loc), geocode_location_get_longitude (loc)); -- GitLab From 75a89c68c368e0ae0c351e65198aafc14ebabdac Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sun, 26 Apr 2020 15:14:07 +0200 Subject: [PATCH 6/8] location: Add accessor function for english sort name --- libgweather/gweather-location.c | 20 ++++++++++++++++++++ libgweather/gweather-location.h | 2 ++ 2 files changed, 22 insertions(+) diff --git a/libgweather/gweather-location.c b/libgweather/gweather-location.c index 1477e51a..c472f274 100644 --- a/libgweather/gweather-location.c +++ b/libgweather/gweather-location.c @@ -616,6 +616,26 @@ gweather_location_get_english_name (GWeatherLocation *loc) return loc->english_name; } +/** + * gweather_location_get_english_sort_name: + * @loc: a #GWeatherLocation + * + * Gets @loc's english "sort name", which is the english name after having + * g_utf8_normalize() (with %G_NORMALIZE_ALL) and g_utf8_casefold() + * called on it. You can use this to sort locations, or to comparing + * user input against a location name. + * + * Return value: @loc's English name for sorting + * Since: 3.38 + **/ +const char * +gweather_location_get_english_sort_name (GWeatherLocation *loc) +{ + g_return_val_if_fail (loc != NULL, NULL); + + return loc->english_sort_name; +} + /** * gweather_location_get_level: * @loc: a #GWeatherLocation diff --git a/libgweather/gweather-location.h b/libgweather/gweather-location.h index b33afb55..701862d8 100644 --- a/libgweather/gweather-location.h +++ b/libgweather/gweather-location.h @@ -65,6 +65,8 @@ const char *gweather_location_get_sort_name (GWeatherLocation *loc) GWEATHER_EXTERN const char *gweather_location_get_english_name (GWeatherLocation *loc); GWEATHER_EXTERN +const char *gweather_location_get_english_sort_name (GWeatherLocation *loc); +GWEATHER_EXTERN GWeatherLocationLevel gweather_location_get_level (GWeatherLocation *loc); GWEATHER_EXTERN GWeatherLocation *gweather_location_get_parent (GWeatherLocation *loc); -- GitLab From 16028d23b2745fa8e78042a4e4aaf7344bdde29c Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sun, 26 Apr 2020 15:14:41 +0200 Subject: [PATCH 7/8] location-entry: Replace struct accesses with function calls This means the strings can be allocated/generated lazily when needed rather than doing that when loading the object. --- libgweather/gweather-location-entry.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/libgweather/gweather-location-entry.c b/libgweather/gweather-location-entry.c index 221a0f93..92c3ce63 100644 --- a/libgweather/gweather-location-entry.c +++ b/libgweather/gweather-location-entry.c @@ -319,7 +319,7 @@ set_location_internal (GWeatherLocationEntry *entry, g_free (name); } else if (loc) { priv->location = gweather_location_ref (loc); - gtk_entry_set_text (GTK_ENTRY (entry), loc->local_name); + gtk_entry_set_text (GTK_ENTRY (entry), gweather_location_get_name (loc)); priv->custom_text = FALSE; } else { priv->location = NULL; @@ -514,9 +514,9 @@ fill_location_entry_model (GtkTreeStore *store, GWeatherLocation *loc, /* Recurse, initializing the names to the country name */ for (i = 0; children[i]; i++) { fill_location_entry_model (store, children[i], - loc->local_name, - loc->local_sort_name, - loc->english_sort_name, + gweather_location_get_name (loc), + gweather_location_get_sort_name (loc), + gweather_location_get_english_sort_name (loc), show_named_timezones); } break; @@ -527,9 +527,9 @@ fill_location_entry_model (GtkTreeStore *store, GWeatherLocation *loc, * 'London, United Kingdom' * You shouldn't need to translate this string unless the language has a different comma. */ - display_name = g_strdup_printf (_("%s, %s"), loc->local_name, parent_display_name); - local_compare_name = g_strdup_printf ("%s, %s", loc->local_sort_name, parent_compare_local_name); - english_compare_name = g_strdup_printf ("%s, %s", loc->english_sort_name, parent_compare_english_name); + display_name = g_strdup_printf (_("%s, %s"), gweather_location_get_name (loc), parent_display_name); + local_compare_name = g_strdup_printf ("%s, %s", gweather_location_get_sort_name (loc), parent_compare_local_name); + english_compare_name = g_strdup_printf ("%s, %s", gweather_location_get_english_sort_name (loc), parent_compare_english_name); for (i = 0; children[i]; i++) { fill_location_entry_model (store, children[i], @@ -556,11 +556,11 @@ fill_location_entry_model (GtkTreeStore *store, GWeatherLocation *loc, * You shouldn't need to translate this string unless the language has a different comma. */ display_name = g_strdup_printf (_("%s, %s"), - loc->local_name, parent_display_name); + gweather_location_get_name (loc), parent_display_name); local_compare_name = g_strdup_printf ("%s, %s", - loc->local_sort_name, parent_compare_local_name); + gweather_location_get_sort_name (loc), parent_compare_local_name); english_compare_name = g_strdup_printf ("%s, %s", - loc->english_sort_name, parent_compare_english_name); + gweather_location_get_english_sort_name (loc), parent_compare_english_name); gtk_tree_store_insert_with_values (store, NULL, NULL, -1, LOC_GWEATHER_LOCATION_ENTRY_COL_LOCATION, loc, @@ -578,9 +578,9 @@ fill_location_entry_model (GtkTreeStore *store, GWeatherLocation *loc, if (show_named_timezones) { gtk_tree_store_insert_with_values (store, NULL, NULL, -1, LOC_GWEATHER_LOCATION_ENTRY_COL_LOCATION, loc, - LOC_GWEATHER_LOCATION_ENTRY_COL_DISPLAY_NAME, loc->local_name, - LOC_GWEATHER_LOCATION_ENTRY_COL_LOCAL_COMPARE_NAME, loc->local_sort_name, - LOC_GWEATHER_LOCATION_ENTRY_COL_ENGLISH_COMPARE_NAME, loc->english_sort_name, + LOC_GWEATHER_LOCATION_ENTRY_COL_DISPLAY_NAME, gweather_location_get_name (loc), + LOC_GWEATHER_LOCATION_ENTRY_COL_LOCAL_COMPARE_NAME, gweather_location_get_sort_name (loc), + LOC_GWEATHER_LOCATION_ENTRY_COL_ENGLISH_COMPARE_NAME, gweather_location_get_english_sort_name (loc), -1); } break; -- GitLab From f2535854bcfe4b4cd994282bc0416bc80ff310cb Mon Sep 17 00:00:00 2001 From: Benjamin Berg Date: Sun, 26 Apr 2020 17:22:54 +0200 Subject: [PATCH 8/8] build: Move pkgconfig generation into toplevel This avoids a dependency of the data subdirectory to be included after the src subdirectory. This is done in preparation to support generating an in-memory database for the locations and being able to depend on that from the src subdirectory. --- data/meson.build | 18 ------------------ meson.build | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/data/meson.build b/data/meson.build index 02694138..88e3d2f6 100644 --- a/data/meson.build +++ b/data/meson.build @@ -1,21 +1,3 @@ -pkgconfig.generate( - filebase: 'gweather-3.0', - name: 'GWeather', - description: 'GWeather shared library', - version: meson.project_version(), - libraries: lib_libgweather, - subdirs: 'libgweather-3.0', - requires: [ - 'gtk+-3.0', - ], - requires_private: [ - 'gio-2.0', - 'libsoup-2.4', - 'libxml-2.0', - 'geocode-glib-1.0', - ], -) - if enable_glade_catalog install_data('glade/libgweather.xml', install_dir: glade_catalogdir, diff --git a/meson.build b/meson.build index 98724ea1..9bf8e753 100644 --- a/meson.build +++ b/meson.build @@ -124,3 +124,21 @@ endif subdir('po') subdir('po-locations') meson.add_install_script('meson/meson_post_install.py') + +pkgconfig.generate( + filebase: 'gweather-3.0', + name: 'GWeather', + description: 'GWeather shared library', + version: meson.project_version(), + libraries: lib_libgweather, + subdirs: 'libgweather-3.0', + requires: [ + 'gtk+-3.0', + ], + requires_private: [ + 'gio-2.0', + 'libsoup-2.4', + 'libxml-2.0', + 'geocode-glib-1.0', + ], +) -- GitLab