From 9b55841d78fbfebf0efd335cfa49927ce1646a97 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 8 Jun 2022 15:45:27 +0200 Subject: [PATCH 1/4] liststore: Use g_object_class_install_properties() Not very useful yet, but future commits add more properties. --- gio/gliststore.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gio/gliststore.c b/gio/gliststore.c index bab3c8e15..555b4f310 100644 --- a/gio/gliststore.c +++ b/gio/gliststore.c @@ -72,6 +72,8 @@ static void g_list_store_iface_init (GListModelInterface *iface); G_DEFINE_TYPE_WITH_CODE (GListStore, g_list_store, G_TYPE_OBJECT, G_IMPLEMENT_INTERFACE (G_TYPE_LIST_MODEL, g_list_store_iface_init)); +static GParamSpec *properties[N_PROPERTIES] = { NULL, }; + static void g_list_store_items_changed (GListStore *store, guint position, @@ -155,9 +157,11 @@ g_list_store_class_init (GListStoreClass *klass) * * Since: 2.44 **/ - g_object_class_install_property (object_class, PROP_ITEM_TYPE, + properties[PROP_ITEM_TYPE] = g_param_spec_gtype ("item-type", "", "", G_TYPE_OBJECT, - G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS)); + G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + + g_object_class_install_properties (object_class, N_PROPERTIES, properties); } static GType From 6b62d63d36a107a7465c4f578ba2892b4747fa87 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 8 Jun 2022 18:19:01 +0200 Subject: [PATCH 2/4] liststore: Add "n-items" property --- gio/gliststore.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/gio/gliststore.c b/gio/gliststore.c index 555b4f310..e7dbbd38f 100644 --- a/gio/gliststore.c +++ b/gio/gliststore.c @@ -64,6 +64,7 @@ enum { PROP_0, PROP_ITEM_TYPE, + PROP_N_ITEMS, N_PROPERTIES }; @@ -89,6 +90,8 @@ g_list_store_items_changed (GListStore *store, } g_list_model_items_changed (G_LIST_MODEL (store), position, removed, added); + if (removed != added) + g_object_notify_by_pspec (G_OBJECT (store), properties[PROP_N_ITEMS]); } static void @@ -115,6 +118,10 @@ g_list_store_get_property (GObject *object, g_value_set_gtype (value, store->item_type); break; + case PROP_N_ITEMS: + g_value_set_uint (value, g_sequence_get_length (store->items)); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); } @@ -157,10 +164,21 @@ g_list_store_class_init (GListStoreClass *klass) * * Since: 2.44 **/ - properties[PROP_ITEM_TYPE] = + properties[PROP_ITEM_TYPE] = g_param_spec_gtype ("item-type", "", "", G_TYPE_OBJECT, G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + /** + * GListStore:n-items: + * + * The number of items contained in this list store. + * + * Since: 2.74 + **/ + properties[PROP_N_ITEMS] = + g_param_spec_uint ("n-items", "", "", 0, G_MAXUINT, 0, + G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + g_object_class_install_properties (object_class, N_PROPERTIES, properties); } From 51840393b2633d752a5ac43da86388b1973d73fd Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Wed, 8 Jun 2022 18:19:29 +0200 Subject: [PATCH 3/4] liststore: Add tests for n-items property --- gio/tests/glistmodel.c | 89 +++++++++++++++++++++++++++++------------- 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/gio/tests/glistmodel.c b/gio/tests/glistmodel.c index d7bf2b51d..f771df81f 100644 --- a/gio/tests/glistmodel.c +++ b/gio/tests/glistmodel.c @@ -38,6 +38,13 @@ list_model_get (GListModel *model, return g_steal_pointer (&item); } +#define assert_cmpitems(store, cmp, n_items) G_STMT_START{ \ + guint tmp; \ + g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), cmp, n_items); \ + g_object_get (store, "n-items", &tmp, NULL); \ + g_assert_cmpuint (tmp, cmp, n_items); \ +}G_STMT_END + /* Test that constructing/getting/setting properties on a #GListStore works. */ static void test_store_properties (void) @@ -88,12 +95,12 @@ test_store_boundaries (void) /* don't allow inserting an item past the end ... */ g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "*g_sequence*"); g_list_store_insert (store, 1, item); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 0); + assert_cmpitems (store, ==, 0); g_test_assert_expected_messages (); /* ... except exactly at the end */ g_list_store_insert (store, 0, item); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 1); + assert_cmpitems (store, ==, 1); /* remove a non-existing item at exactly the end of the list */ g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "*g_sequence*"); @@ -101,7 +108,7 @@ test_store_boundaries (void) g_test_assert_expected_messages (); g_list_store_remove (store, 0); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 0); + assert_cmpitems (store, ==, 0); /* splice beyond the end of the list */ g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "*position*"); @@ -115,13 +122,13 @@ test_store_boundaries (void) g_list_store_append (store, item); g_list_store_splice (store, 0, 1, (gpointer *) &item, 1); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 1); + assert_cmpitems (store, ==, 1); /* remove more items than exist */ g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL, "*position*"); g_list_store_splice (store, 0, 5, NULL, 0); g_test_assert_expected_messages (); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 1); + assert_cmpitems (store, ==, 1); g_object_unref (store); g_assert_finalize_object (item); @@ -138,7 +145,7 @@ test_store_refcounts (void) store = g_list_store_new (G_TYPE_MENU_ITEM); - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 0); + assert_cmpitems (store, ==, 0); g_assert_null (list_model_get (G_LIST_MODEL (store), 0)); n_items = G_N_ELEMENTS (items); @@ -152,7 +159,7 @@ test_store_refcounts (void) g_assert_nonnull (items[i]); } - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, n_items); + assert_cmpitems (store, ==, n_items); g_assert_null (list_model_get (G_LIST_MODEL (store), n_items)); tmp = list_model_get (G_LIST_MODEL (store), 3); @@ -162,7 +169,7 @@ test_store_refcounts (void) g_list_store_remove (store, 4); g_assert_null (items[4]); n_items--; - g_assert_cmpuint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, n_items); + assert_cmpitems (store, ==, n_items); g_assert_null (list_model_get (G_LIST_MODEL (store), n_items)); g_object_unref (store); @@ -229,7 +236,7 @@ test_store_sorted (void) g_free (str); } - g_assert_cmpint (g_list_model_get_n_items (G_LIST_MODEL (store)), ==, 2000); + assert_cmpitems (store, ==, 2000); for (i = 0; i < 1000; i++) { @@ -286,7 +293,7 @@ test_store_splice_replace_middle (void) /* Add three items through splice */ g_list_store_splice (store, 0, 0, array->pdata, 3); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 3); + assert_cmpitems (store, ==, 3); item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); @@ -300,7 +307,7 @@ test_store_splice_replace_middle (void) /* Replace the middle one with two new items */ g_list_store_splice (store, 1, 1, array->pdata + 3, 2); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 4); + assert_cmpitems (store, ==, 4); item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); @@ -342,7 +349,7 @@ test_store_splice_replace_all (void) /* Add the first two */ g_list_store_splice (store, 0, 0, array->pdata, 2); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 2); + assert_cmpitems (store, ==, 2); item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); g_object_unref (item); @@ -353,7 +360,7 @@ test_store_splice_replace_all (void) /* Replace all with the last two */ g_list_store_splice (store, 0, 2, array->pdata + 2, 2); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 2); + assert_cmpitems (store, ==, 2); item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "3"); g_object_unref (item); @@ -378,7 +385,7 @@ test_store_splice_noop (void) /* splice noop with an empty list */ g_list_store_splice (store, 0, 0, NULL, 0); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 0); + assert_cmpitems (store, ==, 0); /* splice noop with a non-empty list */ item = G_ACTION (g_simple_action_new ("1", NULL)); @@ -386,10 +393,10 @@ test_store_splice_noop (void) g_object_unref (item); g_list_store_splice (store, 0, 0, NULL, 0); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 1); + assert_cmpitems (store, ==, 1); g_list_store_splice (store, 1, 0, NULL, 0); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 1); + assert_cmpitems (store, ==, 1); item = list_model_get (model, 0); g_assert_cmpstr (g_action_get_name (item), ==, "1"); @@ -454,21 +461,21 @@ test_store_splice_remove_multiple (void) g_assert_false (model_array_equal (model, array)); g_ptr_array_remove_range (array, 0, 2); g_assert_true (model_array_equal (model, array)); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 8); + assert_cmpitems (store, ==, 8); /* Remove two in the middle */ g_list_store_splice (store, 2, 2, NULL, 0); g_assert_false (model_array_equal (model, array)); g_ptr_array_remove_range (array, 2, 2); g_assert_true (model_array_equal (model, array)); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 6); + assert_cmpitems (store, ==, 6); /* Remove two at the end */ g_list_store_splice (store, 4, 2, NULL, 0); g_assert_false (model_array_equal (model, array)); g_ptr_array_remove_range (array, 4, 2); g_assert_true (model_array_equal (model, array)); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 4); + assert_cmpitems (store, ==, 4); g_ptr_array_unref (array); g_object_unref (store); @@ -528,24 +535,22 @@ static void test_store_remove_all (void) { GListStore *store; - GListModel *model; GSimpleAction *item; store = g_list_store_new (G_TYPE_SIMPLE_ACTION); - model = G_LIST_MODEL (store); /* Test with an empty list */ g_list_store_remove_all (store); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 0); + assert_cmpitems (store, ==, 0); /* Test with a non-empty list */ item = g_simple_action_new ("42", NULL); g_list_store_append (store, item); g_list_store_append (store, item); g_object_unref (item); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 2); + assert_cmpitems (store, ==, 2); g_list_store_remove_all (store); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 0); + assert_cmpitems (store, ==, 0); g_object_unref (store); } @@ -677,6 +682,7 @@ struct ItemsChangedData guint removed; guint added; gboolean called; + gboolean notified; }; static void @@ -689,6 +695,7 @@ expect_items_changed (struct ItemsChangedData *expected, expected->removed = removed; expected->added = added; expected->called = FALSE; + expected->notified = FALSE; } static void @@ -705,6 +712,15 @@ on_items_changed (GListModel *model, expected->called = TRUE; } +static void +on_notify_n_items (GListModel *model, + GParamSpec *pspec, + struct ItemsChangedData *expected) +{ + g_assert_false (expected->notified); + expected->notified = TRUE; +} + /* Test that all operations on the list emit the items-changed signal */ static void test_store_signal_items_changed (void) @@ -719,11 +735,14 @@ test_store_signal_items_changed (void) g_object_connect (model, "signal::items-changed", on_items_changed, &expected, NULL); + g_object_connect (model, "signal::notify::n-items", + on_notify_n_items, &expected, NULL); /* Emit the signal manually */ expect_items_changed (&expected, 0, 0, 0); g_list_model_items_changed (model, 0, 0, 0); g_assert_true (expected.called); + g_assert_false (expected.notified); /* Append an item */ expect_items_changed (&expected, 0, 0, 1); @@ -731,6 +750,7 @@ test_store_signal_items_changed (void) g_list_store_append (store, item); g_object_unref (item); g_assert_true (expected.called); + g_assert_true (expected.notified); /* Insert an item */ expect_items_changed (&expected, 1, 0, 1); @@ -738,6 +758,7 @@ test_store_signal_items_changed (void) g_list_store_insert (store, 1, item); g_object_unref (item); g_assert_true (expected.called); + g_assert_true (expected.notified); /* Sort the list */ expect_items_changed (&expected, 0, 2, 2); @@ -745,6 +766,7 @@ test_store_signal_items_changed (void) (GCompareDataFunc)list_model_cmp_action_by_name, NULL); g_assert_true (expected.called); + g_assert_false (expected.notified); /* Insert sorted */ expect_items_changed (&expected, 2, 0, 1); @@ -755,25 +777,38 @@ test_store_signal_items_changed (void) NULL); g_object_unref (item); g_assert_true (expected.called); + g_assert_true (expected.notified); /* Remove an item */ expect_items_changed (&expected, 1, 1, 0); g_list_store_remove (store, 1); g_assert_true (expected.called); + g_assert_true (expected.notified); /* Splice */ expect_items_changed (&expected, 0, 2, 1); item = g_simple_action_new ("4", NULL); - g_assert_cmpuint (g_list_model_get_n_items (model), >=, 2); + assert_cmpitems (store, >=, 2); g_list_store_splice (store, 0, 2, (gpointer)&item, 1); g_object_unref (item); g_assert_true (expected.called); + g_assert_true (expected.notified); + + /* Splice to replace */ + expect_items_changed (&expected, 0, 1, 1); + item = g_simple_action_new ("5", NULL); + assert_cmpitems (store, >=, 1); + g_list_store_splice (store, 0, 1, (gpointer)&item, 1); + g_object_unref (item); + g_assert_true (expected.called); + g_assert_false (expected.notified); /* Remove all */ expect_items_changed (&expected, 0, 1, 0); - g_assert_cmpuint (g_list_model_get_n_items (model), ==, 1); + assert_cmpitems (store, ==, 1); g_list_store_remove_all (store); g_assert_true (expected.called); + g_assert_true (expected.notified); g_object_unref (store); } @@ -797,7 +832,7 @@ test_store_past_end (void) g_list_store_append (store, item); g_object_unref (item); - g_assert_cmpint (g_list_model_get_n_items (model), ==, 1); + assert_cmpitems (store, ==, 1); item = g_list_model_get_item (model, 0); g_assert_nonnull (item); g_object_unref (item); From 7a4ec897bc471ea7403bce12ff0035c6f1d9c25d Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Thu, 9 Jun 2022 17:03:09 +0200 Subject: [PATCH 4/4] listmodel: Recommend implementing properties "item-type" and "n-items" should ideally be implemented by listmodels, so document that. --- gio/glistmodel.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/gio/glistmodel.c b/gio/glistmodel.c index 94f183ecc..3f9b85e91 100644 --- a/gio/glistmodel.c +++ b/gio/glistmodel.c @@ -84,6 +84,21 @@ G_DEFINE_INTERFACE (GListModel, g_list_model, G_TYPE_OBJECT) * implementation, but typically it will be from the thread that owns * the [thread-default main context][g-main-context-push-thread-default] * in effect at the time that the model was created. + * + * Over time, it has established itself as good practice for listmodel + * implementations to provide properties `item-type` and `n-items` to + * ease working with them. While it is not required, it is recommended + * that implementations provide these two properties. They should return + * the values of g_list_model_get_item_type() and g_list_model_get_n_items() + * respectively and be defined as such: + * |[ + * properties[PROP_ITEM_TYPE] = + * g_param_spec_gtype ("item-type", "", "", G_TYPE_OBJECT, + * G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + * properties[PROP_N_ITEMS] = + * g_param_spec_uint ("n-items", "", "", 0, G_MAXUINT, 0, + * G_PARAM_READABLE | G_PARAM_STATIC_STRINGS); + * ]| */ /**