[Github-comments] [geany/geany-plugins] 'Debugger' plugin port to GTK3 with GTK2 compatibility (#791)

[Colomban Wendling]

b4n requested changes on this pull request.

Looks mostly OK and harmless, although I'm not fond of the style used to support both GTK versions.
There's a few suspicious thing around the end though.

>  		return;
+
+	g_object_get_property(G_OBJECT(cell), "is-expander", &is_expander_value);
+	is_expander = g_value_get_boolean(&is_expander_value);
+	g_value_unset(&is_expander_value);

You should probably just use the simpler version: `g_object_get(cell, "is-expander", &is_expander, NULL)`

>  	cell_renderer->mode = GTK_CELL_RENDERER_MODE_ACTIVATABLE;
+#endif

Here the GTK3 version would work on GTK2 too

>  
-  GdkPixbuf *GSEAL (pixbuf_enabled);
-  GdkPixbuf *GSEAL (pixbuf_disabled);
-  GdkPixbuf *GSEAL (pixbuf_conditional);
-  GdkPixbuf *GSEAL (pixbuf_file);
+  GdkPixbuf *pixbuf_enabled;
+  GdkPixbuf *pixbuf_disabled;
+  GdkPixbuf *pixbuf_conditional;
+  GdkPixbuf *pixbuf_file;

I guess this would ideally use a private member: the goal of `GSEAL()` during GTK3 transition preparation was to "hide" members to callers by mangling their name.  The "correct" solution is to use a private, opaque, structure.

>  
-  GdkPixbuf *GSEAL (pixbuf_active);
-  GdkPixbuf *GSEAL (pixbuf_highlighted);
+  GdkPixbuf *pixbuf_active;
+  GdkPixbuf *pixbuf_highlighted;

Same for private

> @@ -1048,12 +1068,24 @@ void debug_init(void)
 		    NULL);
 	grantpt(pty_master);
 	unlockpt(pty_master);
+#if GTK_CHECK_VERSION(3, 0, 0)
+	pty = vte_pty_new_foreign_sync(pty_master, NULL, NULL);
+	vte_terminal_set_pty(VTE_TERMINAL(terminal), pty);
+	g_object_unref(pty);
+	scrollbar = gtk_scrollbar_new(GTK_ORIENTATION_VERTICAL, gtk_scrollable_get_vadjustment(GTK_SCROLLABLE(terminal)));
+	gtk_widget_set_can_focus(GTK_WIDGET(scrollbar), FALSE);
+#else
 	vte_terminal_set_pty(VTE_TERMINAL(terminal), pty_master);
 	scrollbar = gtk_vscrollbar_new(GTK_ADJUSTMENT(VTE_TERMINAL(terminal)->adjustment));
 	GTK_WIDGET_UNSET_FLAGS(scrollbar, GTK_CAN_FOCUS);

This line could be using the GTK3 version

>  	}
 	g_list_free(modules);
 	gtk_combo_box_set_active(GTK_COMBO_BOX(debugger_cmb), 0);
 
 	/* arguments */
 	args_frame = gtk_frame_new(_("Command Line Arguments"));
+#if GTK_CHECK_VERSION(3, 0, 0)
+	hbox = gtk_scrolled_window_new(
+		gtk_scrollable_get_hadjustment(GTK_SCROLLABLE(args_textview)),
+		gtk_scrollable_get_vadjustment(GTK_SCROLLABLE(args_textview)));
+	gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(hbox), GTK_POLICY_AUTOMATIC, GTK_POLICY_AUTOMATIC);

This is weird to be so different between versions, if the view should be in a scrolled window, it probably should be in any case, even if it works OK with GTK2 but shows a GTK3 bug.

Also, `args_textview` seems ti be used before assignment a few lines down.

> @@ -279,8 +336,15 @@ static void tpage_create_widgets(void)
 
 	/* environment */
 	env_frame = gtk_frame_new(_("Environment Variables"));
+#if GTK_CHECK_VERSION(3, 0, 0)
+	hbox = gtk_scrolled_window_new(
+		gtk_scrollable_get_hadjustment(GTK_SCROLLABLE(args_textview)),
+		gtk_scrollable_get_vadjustment(GTK_SCROLLABLE(args_textview)));
+	gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(hbox), GTK_POLICY_AUTOMATIC, GTK_POLICY_AUTOMATIC);

same here, and using the adjustment from `args_textview` seem wrong here, shouldn't it be `tree`?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/791#pullrequestreview-181400912
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20181204/c4604753/attachment.html>