GNOME Bugzilla – Bug 771929
search-engine-simple: port to use G_DECLARE* type declarations
Last modified: 2016-10-08 19:39:10 UTC
See bug 771777.
working on it
Created attachment 337172 [details] [review] search-engine-simple: port to G_DECLARE* type declaration Currently we are using the old GObject class declarations, which have two problems. One problem is that we cannot use smart pointers like g_autoptr. The other problem is the boilerplate code generated that makes the code less readable, so harder to understand. To fix this use G_DECLARE* type.
Review of attachment 337172 [details] [review]: Thanks for the patch! It still needs some work. I have pointed out the things I found wrong with it. ::: src/nautilus-search-engine-simple.c @@ +545,3 @@ nautilus_search_engine_simple_init (NautilusSearchEngineSimple *engine) { + engine = G_TYPE_INSTANCE_GET_PRIVATE (engine, NAUTILUS_TYPE_SEARCH_ENGINE_SIMPLE, This should be removed altogether. ::: src/nautilus-search-engine-simple.h @@ +27,1 @@ +#define NAUTILUS_TYPE_SEARCH_ENGINE_SIMPLE (nautilus_search_engine_simple_get_type ()) No tabs, please. @@ +29,1 @@ +G_DECLARE_FINAL_TYPE(NautilusSearchEngineSimple, nautilus_search_engine_simple, NAUTILUS, SEARCH_ENGINE_SIMPLE, GObject); Need a space between the macro name and parens. @@ -45,1 @@ -NautilusSearchEngineSimple* nautilus_search_engine_simple_new (void); This should not be removed.
Some advice to go with the review: build Nautilus with your changes before attaching a patch. Make sure they do not introduce new warnings and the build succeeds.
Created attachment 337194 [details] [review] search-engine-simple: port to G_DECLARE* type declaration Currently we are using the old GObject class declarations, which have two problems. One problem is that we cannot use smart pointers like g_autoptr. The other problem is the boilerplate code generated that makes the code less readable, so harder to understand. To fix this use G_DECLARE* type.
Review of attachment 337194 [details] [review]: Yup, looks good to me now. Just one thing I need to know. ::: src/nautilus-search-engine-simple.c @@ +545,3 @@ nautilus_search_engine_simple_init (NautilusSearchEngineSimple *engine) { + engine->recursive = TRUE; Why add this? The property is set to false by default.
Created attachment 337226 [details] [review] search-engine-simple: port to G_DECLARE* type declaration Currently we are using the old GObject class declarations, which have two problems. One problem is that we cannot use smart pointers like g_autoptr. The other problem is the boilerplate code generated that makes the code less readable, so harder to understand. To fix this use G_DECLARE* type.
Review of attachment 337226 [details] [review]: Oops, I missed some things. Sorry about that. You can also remove query_finished from the struct. Seems to be unused. ::: src/nautilus-search-engine-simple.c @@ +64,1 @@ { Ah, you forgot to add the parent instance field. @@ +545,2 @@ nautilus_search_engine_simple_init (NautilusSearchEngineSimple *engine) { query and active_search should be assigned NULL here.
Created attachment 337253 [details] [review] search-engine-simple: port to G_DECLARE* type declaration Currently we are using the old GObject class declarations, which have two problems. One problem is that we cannot use smart pointers like g_autoptr. The other problem is the boilerplate code generated that makes the code less readable, so harder to understand. To fix this use G_DECLARE* type.
Review of attachment 337253 [details] [review]: Solid work. Thanks!
Attachment 337253 [details] pushed as 38e1379 - search-engine-simple: port to G_DECLARE* type declaration