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

Reply via email to