[Github-comments] [geany/geany-plugins] 'Debugger' plugin port to GTK3 with GTK2 compatibility (#791)
LarsGit223
notifications at xxxxx
Sun Nov 25 17:55:05 UTC 2018
LarsGit223 requested changes on this pull request.
I tested the PR with gtk2 and 3 and so far it seems to work in both versions. The gtk3 build did not throw any gtk deprecation warnings. The gtk2 build did, but that comes from the old code so is out of scope of this PR I think. Some might be fixed if my suggestions in ```tpage.c``` will be applied.
But some code could be improved, see my comments.
> @@ -82,17 +82,28 @@ static void on_execute_until(GtkButton *button, gpointer user_data)
*/
GtkWidget* btnpanel_create(on_toggle cb)
{
+#if GTK_CHECK_VERSION(3, 0, 0)
+ GtkWidget *vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, CP_BUTTONS_PAD);
+
I know it was there in the original code already but I think readability would be improved if this empty line would be removed. Also for the non gtk3 code below (line 91).
> calc_width = (gint) cell->xpad * 2 + pixbuf_width;
calc_height = (gint) cell->ypad * 2 + pixbuf_height;
+#endif
According to the gtk documentation the function ```gtk_cell_renderer_get_padding()``` exists since gtk 2.18. As geany itself requires 2.24 or higher this code could be improved by using the gtk3 part only - for gtk2 and 3.
> calc_width = (gint) cell->xpad * 2 + pixbuf_width;
calc_height = (gint) cell->ypad * 2 + pixbuf_height;
+#endif
The same is true for function ```gtk_cell_renderer_get_alignment()```.
>
cell_renderer_break_icon_get_size (cell, widget, cell_area,
&pix_rect.x,
&pix_rect.y,
&pix_rect.width,
&pix_rect.height);
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gtk_cell_renderer_get_padding(cell, &xpad, &ypad);
+ pix_rect.x += cell_area->x + xpad;
+ pix_rect.y += cell_area->y + ypad;
The 3 lines above could be used for gtk version 2 and 3.
> @@ -136,21 +152,37 @@ static void cell_renderer_frame_icon_get_size(GtkCellRenderer *cell, GtkWidget *
pixbuf_height = MAX (pixbuf_height, gdk_pixbuf_get_height (cellframe->pixbuf_highlighted));
}
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gtk_cell_renderer_get_padding(cell, &xpad, &ypad);
+ calc_width = xpad * 2 + pixbuf_width;
+ calc_height = ypad * 2 + pixbuf_height;
+
+ gtk_cell_renderer_get_alignment(cell, &xalign, &yalign);
+#else
As explained above the functions used in the code block above exist for both gtk versions. So IMHO all gtk3 code in this function could be used for both versions replacing the old gtk2 code.
>
cell_renderer_frame_icon_get_size (cell, widget, cell_area,
&pix_rect.x,
&pix_rect.y,
&pix_rect.width,
&pix_rect.height);
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gtk_cell_renderer_get_padding(cell, &xpad, &ypad);
+ pix_rect.x += cell_area->x + xpad;
+ pix_rect.y += cell_area->y + ypad;
+ pix_rect.width -= xpad * 2;
Here also, this 4 lines could be used in both gtk versions.
> @@ -67,8 +67,13 @@ static GKeyFile *keyfile_project = NULL;
static gboolean debug_config_loading = FALSE;
/* saving thread staff */
+#if GTK_CHECK_VERSION(3, 0, 0)
+static GMutex change_config_mutex;
+static GCond cond;
+#else
This is glib specific code and does not depend on gtk. So a check for the gtk version seems to be wrong here.
> @@ -269,8 +274,13 @@ static void save_to_keyfile(GKeyFile *keyfile)
*/
static gpointer saving_thread_func(gpointer data)
{
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gint64 interval;
+ g_mutex_lock(&change_config_mutex);
+#else
I assume this code could be used for all gtk versions, again, not gtk but glib dependent.
> @@ -308,11 +318,20 @@ static gpointer saving_thread_func(gpointer data)
debug_config_changed = FALSE;
}
+#if GTK_CHECK_VERSION(3, 0, 0)
+ interval = g_get_monotonic_time() + SAVING_INTERVAL;
+#else
No gtk dependant code. It depends on the glib version. What is the reason for this change? If it is an improvement then the code could be used without any check as the function ```g_get_monotonic_time()``` exists since glib 2.28 and geany requires 2.32.
> }
+#if GTK_CHECK_VERSION(3, 0, 0)
+ while (!g_cond_wait_until(&cond, &change_config_mutex, interval));
+ g_mutex_unlock(&change_config_mutex);
This requires glib 2.32 at least, not gtk3.
> }
+#if GTK_CHECK_VERSION(3, 0, 0)
+ while (!g_cond_wait_until(&cond, &change_config_mutex, interval));
+ g_mutex_unlock(&change_config_mutex);
Sorry, I mean ```g_cond_wait_until()``` replacing ```g_cond_timed_wait```.
> @@ -458,21 +493,36 @@ void config_init(void)
g_free(data);
}
+#if GTK_CHECK_VERSION(3, 0, 0)
+ g_mutex_init(&change_config_mutex);
+ g_cond_init(&cond);
+ saving_thread = g_thread_new(NULL, saving_thread_func, NULL);
+#else
Again, depends on glib 2.32 not gtk3.
> @@ -320,7 +324,11 @@ static void on_watch_dragged_callback(GtkWidget *wgt, GdkDragContext *context, i
gpointer userdata)
{
/* string that is dragged */
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gchar *expression = (gchar*)gtk_selection_data_get_data(seldata);
The function ```gtk_selection_data_get_data()``` exists since gtk 2.14 and so could be used for both verions.
> @@ -1066,7 +1105,13 @@ void debug_init(void)
configfile = g_strconcat(geany_data->app->configdir, G_DIR_SEPARATOR_S, "geany.conf", NULL);
g_key_file_load_from_file(config, configfile, G_KEY_FILE_NONE, NULL);
font = utils_get_setting_string(config, "VTE", "font", "Monospace 10");
+#if GTK_CHECK_VERSION(3, 0, 0)
+ fontdesc = pango_font_description_from_string(font);
+ vte_terminal_set_font(VTE_TERMINAL(terminal), fontdesc);
+ pango_font_description_free(fontdesc);
+#else
If you like you can replace this lines with a call to function ```gp_vtecompat_set_font_from_string()``` from the geany-plugins util lib. Requires ```#include <gp_vtecompat.h>```.
> @@ -287,8 +288,13 @@ void dpaned_init(void)
/* setup notebooks */
gtk_notebook_set_scrollable(GTK_NOTEBOOK(debug_notebook_left), TRUE);
gtk_notebook_set_scrollable(GTK_NOTEBOOK(debug_notebook_right), TRUE);
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gtk_notebook_set_group_name(GTK_NOTEBOOK(debug_notebook_left), NOTEBOOK_GROUP_NAME(NOTEBOOK_GROUP));
+ gtk_notebook_set_group_name(GTK_NOTEBOOK(debug_notebook_right), NOTEBOOK_GROUP_NAME(NOTEBOOK_GROUP));
+#else
It seems like the code above should be used for both versions gtk2 and 3 as ```gtk_notebook_set_group_id()``` is deprecated since version 2.10.
> @@ -197,7 +197,11 @@ static void on_render_line(GtkTreeViewColumn *tree_column, GtkCellRenderer *cell
g_object_set(cell, "text", "", NULL);
else
{
+#if GTK_CHECK_VERSION(3, 0, 0)
+ GValue value = G_VALUE_INIT;
Depends on glib 2.30, not on gtk3.
> gtk_box_pack_start(GTK_BOX(root), hbox, FALSE, FALSE, 0);
gtk_box_pack_start(GTK_BOX(hbox), target_label, FALSE, FALSE, 0);
gtk_box_pack_start(GTK_BOX(hbox), target_name, TRUE, TRUE, 0);
gtk_box_pack_start(GTK_BOX(hbox), target_button_browse, FALSE, FALSE, 0);
/* lower hbox */
+#if GTK_CHECK_VERSION(3, 0, 0)
+ hbox = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, SPACING);
+ gtk_box_set_homogeneous(GTK_BOX(hbox), TRUE);
Looks like ```gtk_box_set_homogeneous()``` could be used in both gtk versions?
> @@ -196,27 +223,45 @@ void tpage_pack_widgets(gboolean tabbed)
{
GtkWidget *lbox, *rbox, *hbox;
+#if GTK_CHECK_VERSION(3, 0, 0)
+ root = gtk_box_new(GTK_ORIENTATION_HORIZONTAL, SPACING);
+ gtk_box_set_homogeneous(GTK_BOX(root), TRUE);
Here also the line could be used for both versions maybe.
> gtk_widget_set_size_request(target_button_browse, BROWSE_BUTTON_WIDTH, 0);
g_signal_connect(G_OBJECT(target_button_browse), "clicked", G_CALLBACK (on_target_browse_clicked), NULL);
/* debugger */
debugger_label = gtk_label_new(_("Debugger:"));
+#if GTK_CHECK_VERSION(3, 0, 0)
+ debugger_cmb = gtk_combo_box_text_new();
The function ```gtk_combo_box_text_new()``` exists since gtk version 2.24 so we could use that code for both verions (```gtk_combo_box_new_text()``` is deprecated since 2.4).
> modules = debug_get_modules();
for (iter = modules; iter; iter = iter->next)
{
+#if GTK_CHECK_VERSION(3, 0, 0)
+ gtk_combo_box_text_append_text(GTK_COMBO_BOX_TEXT(debugger_cmb), (gchar*)iter->data);
The function ```gtk_combo_box_text_append_text()``` exists since gtk version 2.24 so we could use that code for both verions (```gtk_combo_box_append_text()``` is deprecated since 2.4).
> @@ -366,7 +442,11 @@ int tpage_get_debug_module_index(void)
*/
gchar* tpage_get_debugger(void)
{
+#if GTK_CHECK_VERSION(3, 0, 0)
+ return gtk_combo_box_text_get_active_text(GTK_COMBO_BOX_TEXT(debugger_cmb));
The function ```gtk_combo_box_text_get_active_text()``` exists since gtk version 2.24 so we could use that code for both versions (```gtk_combo_box_get_active_text()``` is deprecated since 2.6).
--
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-178087309
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.geany.org/pipermail/github-comments/attachments/20181125/15d43bbd/attachment-0001.html>
More information about the Github-comments
mailing list