From c26feed8b2db337d0cf76ab1930afc6f213a48a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 15:51:58 +0200 Subject: [PATCH 01/22] eekboard-context-service: Return early if schema is unavailable This also fixes a leak of GSettingsSchema. --- eekboard/eekboard-context-service.c | 39 +++++++++++++++-------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index c0c023d6..7ed63752 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -244,27 +244,28 @@ eekboard_context_service_init (EekboardContextService *self) self->priv = EEKBOARD_CONTEXT_SERVICE_GET_PRIVATE(self); const char *schema_name = "org.gnome.desktop.input-sources"; GSettingsSchemaSource *ssrc = g_settings_schema_source_get_default(); - if (ssrc) { - GSettingsSchema *schema = g_settings_schema_source_lookup(ssrc, - schema_name, - TRUE); - if (schema) { - // Not referencing the found schema directly, - // because it's not clear how... - self->priv->settings = g_settings_new (schema_name); - gulong conn_id = g_signal_connect(self->priv->settings, "change-event", - G_CALLBACK(settings_handle_layout_changed), - self); - if (conn_id == 0) { - g_warning ("Could not connect to gsettings updates, " - "automatic layout changing unavailable"); - } - } else { - g_warning("Gsettings schema %s is not installed on the system. " - "Layout switching unavailable", schema_name); + g_autoptr(GSettingsSchema) schema = NULL; + + if (!ssrc) { + g_warning("No gsettings schemas installed. Layout switching unavailable."); + return; + } + + schema = g_settings_schema_source_lookup(ssrc, schema_name, TRUE); + if (schema) { + // Not referencing the found schema directly, + // because it's not clear how... + self->priv->settings = g_settings_new (schema_name); + gulong conn_id = g_signal_connect(self->priv->settings, "change-event", + G_CALLBACK(settings_handle_layout_changed), + self); + if (conn_id == 0) { + g_warning ("Could not connect to gsettings updates, " + "automatic layout changing unavailable"); } } else { - g_warning("No gsettings schemas installed. Layout switching unavailable."); + g_warning("Gsettings schema %s is not installed on the system. " + "Layout switching unavailable", schema_name); } } From 306c11f1fd7544fd9ff059da9e2e4c055e77e911 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:15:02 +0200 Subject: [PATCH 02/22] treewide: Use new style function definitions --- eek/eek-renderer.c | 4 ++-- src/outputs.h | 2 +- src/style.h | 2 +- src/ui_manager.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/eek/eek-renderer.c b/eek/eek-renderer.c index ceeeb02d..8e62c560 100644 --- a/eek/eek-renderer.c +++ b/eek/eek-renderer.c @@ -241,7 +241,7 @@ static GType new_type(char *name) { ); } -static GType view_type() { +static GType view_type(void) { static GType type = 0; if (!type) { type = new_type("sq_view"); @@ -249,7 +249,7 @@ static GType view_type() { return type; } -static GType button_type() { +static GType button_type(void) { static GType type = 0; if (!type) { type = new_type("sq_button"); diff --git a/src/outputs.h b/src/outputs.h index e018d037..f6466a70 100644 --- a/src/outputs.h +++ b/src/outputs.h @@ -9,7 +9,7 @@ struct squeek_output_handle { struct squeek_outputs *outputs; }; -struct squeek_outputs *squeek_outputs_new(); +struct squeek_outputs *squeek_outputs_new(void); void squeek_outputs_free(struct squeek_outputs*); void squeek_outputs_register(struct squeek_outputs*, struct wl_output *output); struct squeek_output_handle squeek_outputs_get_current(struct squeek_outputs*); diff --git a/src/style.h b/src/style.h index 6fd7c8b7..fdc7d463 100644 --- a/src/style.h +++ b/src/style.h @@ -2,6 +2,6 @@ #define __STYLE_H #include "gtk/gtk.h" -GtkCssProvider *squeek_load_style(); +GtkCssProvider *squeek_load_style(void); #endif diff --git a/src/ui_manager.h b/src/ui_manager.h index 57d3cc70..b0c64592 100644 --- a/src/ui_manager.h +++ b/src/ui_manager.h @@ -7,7 +7,7 @@ struct ui_manager; -struct ui_manager *squeek_uiman_new(); +struct ui_manager *squeek_uiman_new(void); void squeek_uiman_set_output(struct ui_manager *uiman, struct squeek_output_handle output); uint32_t squeek_uiman_get_perceptual_height(struct ui_manager *uiman); From 9d63b505ece632683cd786cb0c18bde14823a496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:26:16 +0200 Subject: [PATCH 03/22] build: Enable '-Wold-style-definition' '-Wstrict-prototypes' --- meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meson.build b/meson.build index 559bb300..a2435c7c 100644 --- a/meson.build +++ b/meson.build @@ -19,6 +19,8 @@ add_project_arguments( '-Werror=missing-field-initializers', '-Werror=incompatible-pointer-types', '-Werror=int-conversion', + '-Wold-style-definition', + '-Wstrict-prototypes', ], language: 'c' ) From 93e9ce0dd7febe909733b8b949aa4340db2aa7ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:16:13 +0200 Subject: [PATCH 04/22] build: Enable '-Wunused-function' --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index a2435c7c..a599ed2b 100644 --- a/meson.build +++ b/meson.build @@ -21,6 +21,7 @@ add_project_arguments( '-Werror=int-conversion', '-Wold-style-definition', '-Wstrict-prototypes', + '-Wunused-function', ], language: 'c' ) From e15d317488e94ee660f159df23b2b799ff7a0469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:29:46 +0200 Subject: [PATCH 05/22] eekboard-context-service: Drop EEKBOARD_CONTEXT_SERVICE_GET_PRIVATE This fixes the ../eekboard/eekboard-context-service.c:244:13: warning: Deprecated pre-processor symbol, replace with 244 | self->priv = EEKBOARD_CONTEXT_SERVICE_GET_PRIVATE(self); warning and makes us use more modern GObject style --- eekboard/eekboard-context-service.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index 7ed63752..1485896c 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -42,9 +42,6 @@ enum { static guint signals[LAST_SIGNAL] = { 0, }; -#define EEKBOARD_CONTEXT_SERVICE_GET_PRIVATE(obj) \ - (G_TYPE_INSTANCE_GET_PRIVATE ((obj), EEKBOARD_TYPE_CONTEXT_SERVICE, EekboardContextServicePrivate)) - struct _EekboardContextServicePrivate { LevelKeyboard *keyboard; // currently used keyboard GSettings *settings; // Owned reference @@ -241,7 +238,8 @@ eekboard_context_service_class_init (EekboardContextServiceClass *klass) static void eekboard_context_service_init (EekboardContextService *self) { - self->priv = EEKBOARD_CONTEXT_SERVICE_GET_PRIVATE(self); + self->priv = eekboard_context_service_get_instance_private (self); + const char *schema_name = "org.gnome.desktop.input-sources"; GSettingsSchemaSource *ssrc = g_settings_schema_source_get_default(); g_autoptr(GSettingsSchema) schema = NULL; From 0f7ab99da38d2840ab88737af7f78b4e7eeeff3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:31:11 +0200 Subject: [PATCH 06/22] keyboard: Fix warning warning: unused variable: `name` --> /var/scratch/librem5/squeekboard/src/keyboard.rs:195:10 | 195 | for (name, state) in keystates.iter() { | ^^^^ help: consider prefixing with an underscore: `_name` --- src/keyboard.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/keyboard.rs b/src/keyboard.rs index 19cd376c..749d21ad 100644 --- a/src/keyboard.rs +++ b/src/keyboard.rs @@ -192,7 +192,7 @@ pub fn generate_keymap( key {{ [ BackSpace ] }};" )?; - for (name, state) in keystates.iter() { + for (_name, state) in keystates.iter() { if let Action::Submit { text: _, keys } = &state.action { for keysym in keys.iter() { write!( From 4228192bda215dba658d244a1438a55fc04656fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:32:48 +0200 Subject: [PATCH 07/22] layout: Fix warning This fixes warning: unnecessary parentheses around block return value --> /var/scratch/librem5/squeekboard/src/layout.rs:110:13 | 110 | / (point.x > self.x && point.x < self.x + self.width 111 | | && point.y > self.y && point.y < self.y + self.height) | |______________________________________________________________________^ | = note: `#[warn(unused_parens)]` on by default --- src/layout.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/layout.rs b/src/layout.rs index 41515bc5..b1cae8ee 100644 --- a/src/layout.rs +++ b/src/layout.rs @@ -107,8 +107,8 @@ pub mod c { impl Bounds { pub fn contains(&self, point: &Point) -> bool { - (point.x > self.x && point.x < self.x + self.width - && point.y > self.y && point.y < self.y + self.height) + point.x > self.x && point.x < self.x + self.width + && point.y > self.y && point.y < self.y + self.height } } From bd661bd4f4aff92311fd5ca1aa8de21e7ce04b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:36:02 +0200 Subject: [PATCH 08/22] gitlab-ci: Enable --Werror This makes sure we don't have more warnings creeping in --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index dfab3af3..878ca595 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -36,7 +36,7 @@ build_meson: expire_in: 3h script: - apt-get -y build-dep . - - meson . _build/ -Ddepdatadir=/usr/share + - meson . _build/ -Ddepdatadir=/usr/share --werror - ninja -C _build install build_deb: From ca68fc20403700a7aadc238372420e8ffdb42cc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:43:40 +0200 Subject: [PATCH 09/22] eek-keyboard: Don't ignore return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This fixes ../eek/eek-keyboard.c:71:5: warning: ignoring return value of ‘getrandom’ declared with attribute ‘warn_unused_result’ [-Wunused-result] 71 | getrandom(r, 6, GRND_NONBLOCK); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ [17/32] Compiling C object 'src/25a6634@@libsqueekboard@sta/.._eek_eek-renderer.c.o' --- eek/eek-keyboard.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/eek/eek-keyboard.c b/eek/eek-keyboard.c index 292a0879..7d000480 100644 --- a/eek/eek-keyboard.c +++ b/eek/eek-keyboard.c @@ -21,6 +21,7 @@ #include "config.h" #define _XOPEN_SOURCE 500 +#include #include #include #include @@ -68,7 +69,8 @@ level_keyboard_new (struct squeek_layout *layout) g_autofree char *path = strdup("/eek_keymap-XXXXXX"); char *r = &path[strlen(path) - 6]; - getrandom(r, 6, GRND_NONBLOCK); + if (getrandom(r, 6, GRND_NONBLOCK) < 0) + g_error("Failed to get random numbers: %s", strerror(errno)); for (unsigned i = 0; i < 6; i++) { r[i] = (r[i] & 0b1111111) | 0b1000000; // A-z r[i] = r[i] > 'z' ? '?' : r[i]; // The randomizer doesn't need to be good... From 857a91640273c3e97745069a841b2d058bed3e07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:58:29 +0200 Subject: [PATCH 10/22] build: Enable -Winit-self No changes needed --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index a599ed2b..a3143483 100644 --- a/meson.build +++ b/meson.build @@ -19,6 +19,7 @@ add_project_arguments( '-Werror=missing-field-initializers', '-Werror=incompatible-pointer-types', '-Werror=int-conversion', + '-Winit-self', '-Wold-style-definition', '-Wstrict-prototypes', '-Wunused-function', From b197cd839ed7bd630536ec09b60996cc397e13d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 17:59:39 +0200 Subject: [PATCH 11/22] build: Enable -Wformat-security No changes needed. --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index a3143483..4e06a38b 100644 --- a/meson.build +++ b/meson.build @@ -19,6 +19,7 @@ add_project_arguments( '-Werror=missing-field-initializers', '-Werror=incompatible-pointer-types', '-Werror=int-conversion', + '-Wformat-security', '-Winit-self', '-Wold-style-definition', '-Wstrict-prototypes', From 24b6a0490358e02435ef108bbebb7147b99765af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:01:05 +0200 Subject: [PATCH 12/22] build: Enable -Wmaybe-uninitialized No changes needed. --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index 4e06a38b..2ac7782c 100644 --- a/meson.build +++ b/meson.build @@ -21,6 +21,7 @@ add_project_arguments( '-Werror=int-conversion', '-Wformat-security', '-Winit-self', + '-Wmaybe-uninitialized', '-Wold-style-definition', '-Wstrict-prototypes', '-Wunused-function', From eb7673d2c2d5f88993b84dc225c0d38407886f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:07:10 +0200 Subject: [PATCH 13/22] treewide: Drop redundant declarations --- eek/eek-element.h | 2 -- eek/eek-gtk-keyboard.h | 1 - 2 files changed, 3 deletions(-) diff --git a/eek/eek-element.h b/eek/eek-element.h index 87e59723..83b1e7e5 100644 --- a/eek/eek-element.h +++ b/eek/eek-element.h @@ -38,8 +38,6 @@ struct _EekElementClass GObjectClass parent_class; }; -GType eek_element_get_type (void) G_GNUC_CONST; - void eek_element_set_name (EekElement *element, const gchar *name); diff --git a/eek/eek-gtk-keyboard.h b/eek/eek-gtk-keyboard.h index 1f07cba3..e9df72b3 100644 --- a/eek/eek-gtk-keyboard.h +++ b/eek/eek-gtk-keyboard.h @@ -47,7 +47,6 @@ struct _EekGtkKeyboardClass gpointer pdummy[24]; }; -GType eek_gtk_keyboard_get_type (void) G_GNUC_CONST; GtkWidget *eek_gtk_keyboard_new (EekboardContextService *eekservice, struct submission *submission, struct squeek_layout_state *layout); void eek_gtk_keyboard_emit_feedback (EekGtkKeyboard *self); From 6756fb423a650f702706aa2326799c1ad8db8ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:07:20 +0200 Subject: [PATCH 14/22] build: Enable -Wredundant-declarations --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index 2ac7782c..a0e91f5c 100644 --- a/meson.build +++ b/meson.build @@ -23,6 +23,7 @@ add_project_arguments( '-Winit-self', '-Wmaybe-uninitialized', '-Wold-style-definition', + '-Wredundant-decls', '-Wstrict-prototypes', '-Wunused-function', ], From 97f51591b32cb0833f6809d05b39c26a51ffefc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:13:08 +0200 Subject: [PATCH 15/22] ServerContextService: Drop GObject boilerplate G_DECLARE_FINAL_TYPE does this for us --- src/server-context-service.c | 6 ------ src/server-context-service.h | 10 +--------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/src/server-context-service.c b/src/server-context-service.c index 48f1deeb..f8033543 100644 --- a/src/server-context-service.c +++ b/src/server-context-service.c @@ -34,8 +34,6 @@ enum { PROP_LAST }; -typedef struct _ServerContextServiceClass ServerContextServiceClass; - struct _ServerContextService { GObject parent; @@ -52,10 +50,6 @@ struct _ServerContextService { guint last_requested_height; }; -struct _ServerContextServiceClass { - GObjectClass parent_class; -}; - G_DEFINE_TYPE(ServerContextService, server_context_service, G_TYPE_OBJECT); static void diff --git a/src/server-context-service.h b/src/server-context-service.h index a69f85ae..de6b8dc8 100644 --- a/src/server-context-service.h +++ b/src/server-context-service.h @@ -25,17 +25,9 @@ G_BEGIN_DECLS #define SERVER_TYPE_CONTEXT_SERVICE (server_context_service_get_type()) -#define SERVER_CONTEXT_SERVICE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), SERVER_TYPE_CONTEXT_SERVICE, ServerContextService)) -#define SERVER_CONTEXT_SERVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), SERVER_TYPE_CONTEXT_SERVICE, ServerContextServiceClass)) -#define SERVER_IS_CONTEXT_SERVICE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), SERVER_TYPE_CONTEXT_SERVICE)) -#define SERVER_IS_CONTEXT_SERVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), SERVER_TYPE_CONTEXT_SERVICE)) -#define SERVER_CONTEXT_SERVICE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SERVER_TYPE_CONTEXT_SERVICE, ServerContextServiceClass)) /** Manages the lifecycle of the window displaying layouts. */ -typedef struct _ServerContextService ServerContextService; - -GType server_context_service_get_type - (void) G_GNUC_CONST; +G_DECLARE_FINAL_TYPE (ServerContextService, server_context_service, SERVER, CONTEXT_SERVICE, GObject) ServerContextService *server_context_service_new(EekboardContextService *state, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman); enum squeek_arrangement_kind server_context_service_get_layout_type(ServerContextService *); From a8b81172fc0197fc3c6f41d3f79ce54df11873cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:40:56 +0200 Subject: [PATCH 16/22] build: Enable '-Wformat-nonliteral' --- meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/meson.build b/meson.build index a0e91f5c..d8e0e1bb 100644 --- a/meson.build +++ b/meson.build @@ -19,6 +19,7 @@ add_project_arguments( '-Werror=missing-field-initializers', '-Werror=incompatible-pointer-types', '-Werror=int-conversion', + '-Wformat-nonliteral', '-Wformat-security', '-Winit-self', '-Wmaybe-uninitialized', From 966990ad6512058c80196d3a4a1ebf937acde729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:45:33 +0200 Subject: [PATCH 17/22] eekboad-context-service: Drop signal class handler It's unused --- eekboard/eekboard-context-service.c | 2 +- eekboard/eekboard-context-service.h | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index 1485896c..a90bf707 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -214,7 +214,7 @@ eekboard_context_service_class_init (EekboardContextServiceClass *klass) g_signal_new (I_("destroyed"), G_TYPE_FROM_CLASS(gobject_class), G_SIGNAL_RUN_LAST, - G_STRUCT_OFFSET(EekboardContextServiceClass, destroyed), + 0, NULL, NULL, g_cclosure_marshal_VOID__VOID, diff --git a/eekboard/eekboard-context-service.h b/eekboard/eekboard-context-service.h index b1d47f42..96f4f240 100644 --- a/eekboard/eekboard-context-service.h +++ b/eekboard/eekboard-context-service.h @@ -71,10 +71,6 @@ struct _EekboardContextServiceClass { /*< private >*/ GObjectClass parent_class; - /*< public >*/ - /* signals */ - void (*destroyed) (EekboardContextService *self); - /*< private >*/ /* padding */ gpointer pdummy[24]; From 3e212ddab41d34278f243eed2df2b89eaa27bc70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:46:22 +0200 Subject: [PATCH 18/22] eekboard-context-service: Drop docstrings for inexistent functions --- eekboard/eekboard-context-service.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/eekboard/eekboard-context-service.h b/eekboard/eekboard-context-service.h index 96f4f240..7a506786 100644 --- a/eekboard/eekboard-context-service.h +++ b/eekboard/eekboard-context-service.h @@ -63,9 +63,6 @@ struct _EekboardContextService { /** * EekboardContextServiceClass: - * @create_keyboard: virtual function for create a keyboard from string - * @enabled: class handler for #EekboardContextService::enabled signal - * @disabled: class handler for #EekboardContextService::disabled signal */ struct _EekboardContextServiceClass { /*< private >*/ From 53f30324f02deb0a56d4c3e02e41effc742ecd1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:51:21 +0200 Subject: [PATCH 19/22] eekboard-context-service: Drop the GObject boilerplate The previous commits show it's not really a derivable type so make it a finale one. --- eekboard/eekboard-context-service.c | 17 +++++++++++++ eekboard/eekboard-context-service.h | 39 +---------------------------- 2 files changed, 18 insertions(+), 38 deletions(-) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index a90bf707..e9a23d8b 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -42,6 +42,23 @@ enum { static guint signals[LAST_SIGNAL] = { 0, }; +/** + * EekboardContextService: + * + * Handles layout state, gsettings, and virtual-keyboard. + * + * TODO: Restrict to managing keyboard layouts, and maybe button repeats, + * and the virtual keyboard protocol. + * + * The #EekboardContextService structure contains only private data + * and should only be accessed using the provided API. + */ +struct _EekboardContextService { + GObject parent; + EekboardContextServicePrivate *priv; + struct squeek_layout_state *layout; // Unowned +}; + struct _EekboardContextServicePrivate { LevelKeyboard *keyboard; // currently used keyboard GSettings *settings; // Owned reference diff --git a/eekboard/eekboard-context-service.h b/eekboard/eekboard-context-service.h index 7a506786..4c310b03 100644 --- a/eekboard/eekboard-context-service.h +++ b/eekboard/eekboard-context-service.h @@ -34,47 +34,10 @@ G_BEGIN_DECLS #define EEKBOARD_CONTEXT_SERVICE_INTERFACE "org.fedorahosted.Eekboard.Context" #define EEKBOARD_TYPE_CONTEXT_SERVICE (eekboard_context_service_get_type()) -#define EEKBOARD_CONTEXT_SERVICE(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), EEKBOARD_TYPE_CONTEXT_SERVICE, EekboardContextService)) -#define EEKBOARD_CONTEXT_SERVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), EEKBOARD_TYPE_CONTEXT_SERVICE, EekboardContextServiceClass)) -#define EEKBOARD_IS_CONTEXT_SERVICE(obj) (G_TYPE_CHECK_INSTANCE_TYPE ((obj), EEKBOARD_TYPE_CONTEXT_SERVICE)) -#define EEKBOARD_IS_CONTEXT_SERVICE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), EEKBOARD_TYPE_CONTEXT_SERVICE)) -#define EEKBOARD_CONTEXT_SERVICE_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), EEKBOARD_TYPE_CONTEXT_SERVICE, EekboardContextServiceClass)) - -typedef struct _EekboardContextServiceClass EekboardContextServiceClass; typedef struct _EekboardContextServicePrivate EekboardContextServicePrivate; +G_DECLARE_FINAL_TYPE(EekboardContextService, eekboard_context_service, EEKBOARD, CONTEXT_SERVICE, GObject) -/** - * EekboardContextService: - * - * Handles layout state, gsettings, and virtual-keyboard. - * - * TODO: Restrict to managing keyboard layouts, and maybe button repeats, - * and the virtual keyboard protocol. - * - * The #EekboardContextService structure contains only private data - * and should only be accessed using the provided API. - */ -struct _EekboardContextService { - GObject parent; - EekboardContextServicePrivate *priv; - struct squeek_layout_state *layout; // Unowned -}; - -/** - * EekboardContextServiceClass: - */ -struct _EekboardContextServiceClass { - /*< private >*/ - GObjectClass parent_class; - - /*< private >*/ - /* padding */ - gpointer pdummy[24]; -}; - -GType eekboard_context_service_get_type - (void) G_GNUC_CONST; EekboardContextService *eekboard_context_service_new(struct squeek_layout_state *state); void eekboard_context_service_set_submission(EekboardContextService *context, struct submission *submission); void eekboard_context_service_set_ui(EekboardContextService *context, ServerContextService *ui); From 07faf906d8bc2e4c0c135160ac73c075c6ad2c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 18:53:22 +0200 Subject: [PATCH 20/22] eekboard-context-service: Drop private struct There's no point having it for a final type and it only makes the code harder to read. --- eekboard/eekboard-context-service.c | 33 ++++++++++++----------------- eekboard/eekboard-context-service.h | 1 - 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/eekboard/eekboard-context-service.c b/eekboard/eekboard-context-service.c index e9a23d8b..a360c2b6 100644 --- a/eekboard/eekboard-context-service.c +++ b/eekboard/eekboard-context-service.c @@ -55,11 +55,8 @@ static guint signals[LAST_SIGNAL] = { 0, }; */ struct _EekboardContextService { GObject parent; - EekboardContextServicePrivate *priv; struct squeek_layout_state *layout; // Unowned -}; -struct _EekboardContextServicePrivate { LevelKeyboard *keyboard; // currently used keyboard GSettings *settings; // Owned reference @@ -70,7 +67,7 @@ struct _EekboardContextServicePrivate { struct submission *submission; // unowned }; -G_DEFINE_TYPE_WITH_PRIVATE (EekboardContextService, eekboard_context_service, G_TYPE_OBJECT); +G_DEFINE_TYPE (EekboardContextService, eekboard_context_service, G_TYPE_OBJECT); static void eekboard_context_service_set_property (GObject *object, @@ -96,7 +93,7 @@ eekboard_context_service_get_property (GObject *object, switch (prop_id) { case PROP_KEYBOARD: - g_value_set_object (value, context->priv->keyboard); + g_value_set_object (value, context->keyboard); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -157,12 +154,12 @@ eekboard_context_service_use_layout(EekboardContextService *context, struct sque struct squeek_layout *layout = squeek_load_layout(layout_name, state->arrangement); LevelKeyboard *keyboard = level_keyboard_new(layout); // set as current - LevelKeyboard *previous_keyboard = context->priv->keyboard; - context->priv->keyboard = keyboard; + LevelKeyboard *previous_keyboard = context->keyboard; + context->keyboard = keyboard; // Update the keymap if necessary. // TODO: Update submission on change event - if (context->priv->submission) { - submission_set_keyboard(context->priv->submission, keyboard, timestamp); + if (context->submission) { + submission_set_keyboard(context->submission, keyboard, timestamp); } // Update UI @@ -177,7 +174,7 @@ eekboard_context_service_use_layout(EekboardContextService *context, struct sque static void eekboard_context_service_update_settings_layout(EekboardContextService *context) { g_autofree gchar *keyboard_layout = NULL; g_autofree gchar *keyboard_type = NULL; - settings_get_layout(context->priv->settings, + settings_get_layout(context->settings, &keyboard_type, &keyboard_layout); if (g_strcmp0(context->layout->layout_name, keyboard_layout) != 0 || context->layout->overlay_name) { @@ -255,8 +252,6 @@ eekboard_context_service_class_init (EekboardContextServiceClass *klass) static void eekboard_context_service_init (EekboardContextService *self) { - self->priv = eekboard_context_service_get_instance_private (self); - const char *schema_name = "org.gnome.desktop.input-sources"; GSettingsSchemaSource *ssrc = g_settings_schema_source_get_default(); g_autoptr(GSettingsSchema) schema = NULL; @@ -270,8 +265,8 @@ eekboard_context_service_init (EekboardContextService *self) if (schema) { // Not referencing the found schema directly, // because it's not clear how... - self->priv->settings = g_settings_new (schema_name); - gulong conn_id = g_signal_connect(self->priv->settings, "change-event", + self->settings = g_settings_new (schema_name); + gulong conn_id = g_signal_connect(self->settings, "change-event", G_CALLBACK(settings_handle_layout_changed), self); if (conn_id == 0) { @@ -307,7 +302,7 @@ eekboard_context_service_destroy (EekboardContextService *context) LevelKeyboard * eekboard_context_service_get_keyboard (EekboardContextService *context) { - return context->priv->keyboard; + return context->keyboard; } void eekboard_context_service_set_hint_purpose(EekboardContextService *context, @@ -347,13 +342,13 @@ EekboardContextService *eekboard_context_service_new(struct squeek_layout_state } void eekboard_context_service_set_submission(EekboardContextService *context, struct submission *submission) { - context->priv->submission = submission; - if (context->priv->submission) { + context->submission = submission; + if (context->submission) { uint32_t time = gdk_event_get_time(NULL); - submission_set_keyboard(context->priv->submission, context->priv->keyboard, time); + submission_set_keyboard(context->submission, context->keyboard, time); } } void eekboard_context_service_set_ui(EekboardContextService *context, ServerContextService *ui) { - context->priv->ui = ui; + context->ui = ui; } diff --git a/eekboard/eekboard-context-service.h b/eekboard/eekboard-context-service.h index 4c310b03..345c246e 100644 --- a/eekboard/eekboard-context-service.h +++ b/eekboard/eekboard-context-service.h @@ -35,7 +35,6 @@ G_BEGIN_DECLS #define EEKBOARD_TYPE_CONTEXT_SERVICE (eekboard_context_service_get_type()) -typedef struct _EekboardContextServicePrivate EekboardContextServicePrivate; G_DECLARE_FINAL_TYPE(EekboardContextService, eekboard_context_service, EEKBOARD, CONTEXT_SERVICE, GObject) EekboardContextService *eekboard_context_service_new(struct squeek_layout_state *state); From 1e6bcef055ba9853f3c568904cdfb96541ab7344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 20:05:49 +0200 Subject: [PATCH 21/22] server-context-service: Consistenty name self argument 'self' It's confusing when the object a method acts on is sometimes called context and sometimes called state. So name it 'self' as we do in other projects. --- src/server-context-service.c | 148 +++++++++++++++++------------------ src/server-context-service.h | 6 +- 2 files changed, 77 insertions(+), 77 deletions(-) diff --git a/src/server-context-service.c b/src/server-context-service.c index f8033543..ca5b89dd 100644 --- a/src/server-context-service.c +++ b/src/server-context-service.c @@ -55,30 +55,30 @@ G_DEFINE_TYPE(ServerContextService, server_context_service, G_TYPE_OBJECT); static void on_destroy (GtkWidget *widget, gpointer user_data) { - ServerContextService *context = user_data; + ServerContextService *self = user_data; - g_assert (widget == GTK_WIDGET(context->window)); + g_assert (widget == GTK_WIDGET(self->window)); - context->window = NULL; - context->widget = NULL; + self->window = NULL; + self->widget = NULL; //eekboard_context_service_destroy (EEKBOARD_CONTEXT_SERVICE (context)); } static void on_notify_map (GObject *object, - ServerContextService *context) + ServerContextService *self) { - g_object_set (context, "visible", TRUE, NULL); + (void)object; + g_object_set (self, "visible", TRUE, NULL); } static void on_notify_unmap (GObject *object, - ServerContextService *context) + ServerContextService *self) { - (void)object; - g_object_set (context, "visible", FALSE, NULL); + g_object_set (self, "visible", FALSE, NULL); } static uint32_t @@ -94,7 +94,7 @@ calculate_height(int32_t width) } static void -on_surface_configure(PhoshLayerSurface *surface, ServerContextService *context) +on_surface_configure(PhoshLayerSurface *surface, ServerContextService *self) { gint width; gint height; @@ -116,8 +116,8 @@ on_surface_configure(PhoshLayerSurface *surface, ServerContextService *context) // as it's likely to create pointless loops // of request->reject->request_again->... if (desired_height != configured_height - && context->last_requested_height != desired_height) { - context->last_requested_height = desired_height; + && self->last_requested_height != desired_height) { + self->last_requested_height = desired_height; phosh_layer_surface_set_size(surface, 0, (gint)desired_height); phosh_layer_surface_set_exclusive_zone(surface, (gint)desired_height); @@ -126,16 +126,16 @@ on_surface_configure(PhoshLayerSurface *surface, ServerContextService *context) } static void -make_window (ServerContextService *context) +make_window (ServerContextService *self) { - if (context->window) + if (self->window) g_error("Window already present"); struct squeek_output_handle output = squeek_outputs_get_current(squeek_wayland->outputs); - squeek_uiman_set_output(context->manager, output); - uint32_t height = squeek_uiman_get_perceptual_height(context->manager); + squeek_uiman_set_output(self->manager, output); + uint32_t height = squeek_uiman_get_perceptual_height(self->manager); - context->window = g_object_new ( + self->window = g_object_new ( PHOSH_TYPE_LAYER_SURFACE, "layer-shell", squeek_wayland->layer_shell, "wl-output", output.output, @@ -150,11 +150,11 @@ make_window (ServerContextService *context) NULL ); - g_object_connect (context->window, - "signal::destroy", G_CALLBACK(on_destroy), context, - "signal::map", G_CALLBACK(on_notify_map), context, - "signal::unmap", G_CALLBACK(on_notify_unmap), context, - "signal::configured", G_CALLBACK(on_surface_configure), context, + g_object_connect (self->window, + "signal::destroy", G_CALLBACK(on_destroy), self, + "signal::map", G_CALLBACK(on_notify_map), self, + "signal::unmap", G_CALLBACK(on_notify_unmap), self, + "signal::configured", G_CALLBACK(on_surface_configure), self, NULL); // The properties below are just to make hacking easier. @@ -162,87 +162,87 @@ make_window (ServerContextService *context) // and there's no space in the protocol for others. // Those may still be useful in the future, // or for hacks with regular windows. - gtk_widget_set_can_focus (GTK_WIDGET(context->window), FALSE); - g_object_set (G_OBJECT(context->window), "accept_focus", FALSE, NULL); - gtk_window_set_title (GTK_WINDOW(context->window), + gtk_widget_set_can_focus (GTK_WIDGET(self->window), FALSE); + g_object_set (G_OBJECT(self->window), "accept_focus", FALSE, NULL); + gtk_window_set_title (GTK_WINDOW(self->window), _("Squeekboard")); - gtk_window_set_icon_name (GTK_WINDOW(context->window), "squeekboard"); - gtk_window_set_keep_above (GTK_WINDOW(context->window), TRUE); + gtk_window_set_icon_name (GTK_WINDOW(self->window), "squeekboard"); + gtk_window_set_keep_above (GTK_WINDOW(self->window), TRUE); } static void -destroy_window (ServerContextService *context) +destroy_window (ServerContextService *self) { - gtk_widget_destroy (GTK_WIDGET (context->window)); - context->window = NULL; + gtk_widget_destroy (GTK_WIDGET (self->window)); + self->window = NULL; } static void -make_widget (ServerContextService *context) +make_widget (ServerContextService *self) { - if (context->widget) { - gtk_widget_destroy(context->widget); - context->widget = NULL; + if (self->widget) { + gtk_widget_destroy(self->widget); + self->widget = NULL; } - context->widget = eek_gtk_keyboard_new (context->state, context->submission, context->layout); + self->widget = eek_gtk_keyboard_new (self->state, self->submission, self->layout); - gtk_widget_set_has_tooltip (context->widget, TRUE); - gtk_container_add (GTK_CONTAINER(context->window), context->widget); - gtk_widget_show_all(context->widget); + gtk_widget_set_has_tooltip (self->widget, TRUE); + gtk_container_add (GTK_CONTAINER(self->window), self->widget); + gtk_widget_show_all(self->widget); } static gboolean -on_hide (ServerContextService *context) +on_hide (ServerContextService *self) { - gtk_widget_hide (GTK_WIDGET(context->window)); - context->hiding = 0; + gtk_widget_hide (GTK_WIDGET(self->window)); + self->hiding = 0; return G_SOURCE_REMOVE; } static void -server_context_service_real_show_keyboard (ServerContextService *context) +server_context_service_real_show_keyboard (ServerContextService *self) { - if (context->hiding) { - g_source_remove (context->hiding); - context->hiding = 0; + if (self->hiding) { + g_source_remove (self->hiding); + self->hiding = 0; } - if (!context->window) - make_window (context); - if (!context->widget) - make_widget (context); + if (!self->window) + make_window (self); + if (!self->widget) + make_widget (self); - context->visible = TRUE; - gtk_widget_show (GTK_WIDGET(context->window)); + self->visible = TRUE; + gtk_widget_show (GTK_WIDGET(self->window)); } static void -server_context_service_real_hide_keyboard (ServerContextService *context) +server_context_service_real_hide_keyboard (ServerContextService *self) { - if (!context->hiding) - context->hiding = g_timeout_add (200, (GSourceFunc) on_hide, context); + if (!self->hiding) + self->hiding = g_timeout_add (200, (GSourceFunc) on_hide, self); - context->visible = FALSE; + self->visible = FALSE; } void -server_context_service_show_keyboard (ServerContextService *context) +server_context_service_show_keyboard (ServerContextService *self) { - g_return_if_fail (SERVER_IS_CONTEXT_SERVICE(context)); + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE(self)); - if (!context->visible) { - server_context_service_real_show_keyboard (context); + if (!self->visible) { + server_context_service_real_show_keyboard (self); } } void -server_context_service_hide_keyboard (ServerContextService *context) +server_context_service_hide_keyboard (ServerContextService *self) { - g_return_if_fail (SERVER_IS_CONTEXT_SERVICE(context)); + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE(self)); - if (context->visible) { - server_context_service_real_hide_keyboard (context); + if (self->visible) { + server_context_service_real_hide_keyboard (self); } } @@ -252,11 +252,11 @@ server_context_service_set_property (GObject *object, const GValue *value, GParamSpec *pspec) { - ServerContextService *context = SERVER_CONTEXT_SERVICE(object); + ServerContextService *self = SERVER_CONTEXT_SERVICE(object); switch (prop_id) { case PROP_VISIBLE: - context->visible = g_value_get_boolean (value); + self->visible = g_value_get_boolean (value); break; default: @@ -271,10 +271,10 @@ server_context_service_get_property (GObject *object, GValue *value, GParamSpec *pspec) { - ServerContextService *context = SERVER_CONTEXT_SERVICE(object); + ServerContextService *self = SERVER_CONTEXT_SERVICE(object); switch (prop_id) { case PROP_VISIBLE: - g_value_set_boolean (value, context->visible); + g_value_set_boolean (value, self->visible); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); @@ -285,10 +285,10 @@ server_context_service_get_property (GObject *object, static void server_context_service_dispose (GObject *object) { - ServerContextService *context = SERVER_CONTEXT_SERVICE(object); + ServerContextService *self = SERVER_CONTEXT_SERVICE(object); - destroy_window (context); - context->widget = NULL; + destroy_window (self); + self->widget = NULL; G_OBJECT_CLASS (server_context_service_parent_class)->dispose (object); } @@ -317,16 +317,16 @@ server_context_service_class_init (ServerContextServiceClass *klass) } static void -server_context_service_init (ServerContextService *state) { - (void)state; +server_context_service_init (ServerContextService *self) { + (void)self; } ServerContextService * -server_context_service_new (EekboardContextService *state, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman) +server_context_service_new (EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman) { ServerContextService *ui = g_object_new (SERVER_TYPE_CONTEXT_SERVICE, NULL); ui->submission = submission; - ui->state = state; + ui->state = self; ui->layout = layout; ui->manager = uiman; return ui; diff --git a/src/server-context-service.h b/src/server-context-service.h index de6b8dc8..bafe71c3 100644 --- a/src/server-context-service.h +++ b/src/server-context-service.h @@ -29,10 +29,10 @@ G_BEGIN_DECLS /** Manages the lifecycle of the window displaying layouts. */ G_DECLARE_FINAL_TYPE (ServerContextService, server_context_service, SERVER, CONTEXT_SERVICE, GObject) -ServerContextService *server_context_service_new(EekboardContextService *state, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman); +ServerContextService *server_context_service_new(EekboardContextService *self, struct submission *submission, struct squeek_layout_state *layout, struct ui_manager *uiman); enum squeek_arrangement_kind server_context_service_get_layout_type(ServerContextService *); -void server_context_service_show_keyboard (ServerContextService *context); -void server_context_service_hide_keyboard (ServerContextService *context); +void server_context_service_show_keyboard (ServerContextService *self); +void server_context_service_hide_keyboard (ServerContextService *self); G_END_DECLS #endif /* SERVER_CONTEXT_SERVICE_H */ From 8bdfb69dc1b057476ee7079573591db702c932c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= Date: Fri, 11 Sep 2020 20:12:02 +0200 Subject: [PATCH 22/22] server-context-service: swap signal arguments This makes sure 'self' comes first. While at that fix the function signatures and use ServerContextService directly and add type checks so it's easy to notice when we messed up. --- src/server-context-service.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/server-context-service.c b/src/server-context-service.c index ca5b89dd..ded432ef 100644 --- a/src/server-context-service.c +++ b/src/server-context-service.c @@ -53,9 +53,9 @@ struct _ServerContextService { G_DEFINE_TYPE(ServerContextService, server_context_service, G_TYPE_OBJECT); static void -on_destroy (GtkWidget *widget, gpointer user_data) +on_destroy (ServerContextService *self, GtkWidget *widget) { - ServerContextService *self = user_data; + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); g_assert (widget == GTK_WIDGET(self->window)); @@ -66,18 +66,19 @@ on_destroy (GtkWidget *widget, gpointer user_data) } static void -on_notify_map (GObject *object, - ServerContextService *self) +on_notify_map (ServerContextService *self, GtkWidget *widget) { - (void)object; + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); + g_object_set (self, "visible", TRUE, NULL); } static void -on_notify_unmap (GObject *object, - ServerContextService *self) +on_notify_unmap (ServerContextService *self, GtkWidget *widget) { + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); + g_object_set (self, "visible", FALSE, NULL); } @@ -94,10 +95,14 @@ calculate_height(int32_t width) } static void -on_surface_configure(PhoshLayerSurface *surface, ServerContextService *self) +on_surface_configure(ServerContextService *self, PhoshLayerSurface *surface) { gint width; gint height; + + g_return_if_fail (SERVER_IS_CONTEXT_SERVICE (self)); + g_return_if_fail (PHOSH_IS_LAYER_SURFACE (surface)); + g_object_get(G_OBJECT(surface), "configured-width", &width, "configured-height", &height, @@ -151,10 +156,10 @@ make_window (ServerContextService *self) ); g_object_connect (self->window, - "signal::destroy", G_CALLBACK(on_destroy), self, - "signal::map", G_CALLBACK(on_notify_map), self, - "signal::unmap", G_CALLBACK(on_notify_unmap), self, - "signal::configured", G_CALLBACK(on_surface_configure), self, + "swapped-signal::destroy", G_CALLBACK(on_destroy), self, + "swapped-signal::map", G_CALLBACK(on_notify_map), self, + "swapped-signal::unmap", G_CALLBACK(on_notify_unmap), self, + "swapped-signal::configured", G_CALLBACK(on_surface_configure), self, NULL); // The properties below are just to make hacking easier.