This pull request is heavily based on #645 from which it borrows the majority of the changes. The main improvement is that this one doesn't break compatibility with GTK2 (there are macros which select the correct code path accordingly). I also removed some cosmetic-only changes to maintain the differences as small as possible and provided a corrected code for console font selection (which was just commented out in the original pull request).
This was successfully tested on Centos 7.5 with stock development packages. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/791
-- Commit Summary --
* Ported the 'Debugger' plugin to GTK3 while maintaining compatibility with GTK2.
-- File Changes --
M build/debugger.m4 (5) M debugger/src/btnpanel.c (25) M debugger/src/cell_renderers/cellrendererbreakicon.c (73) M debugger/src/cell_renderers/cellrendererbreakicon.h (11) M debugger/src/cell_renderers/cellrendererframeicon.c (62) M debugger/src/cell_renderers/cellrendererframeicon.h (7) M debugger/src/cell_renderers/cellrenderertoggle.c (9) M debugger/src/dconfig.c (68) M debugger/src/debug.c (49) M debugger/src/dpaned.c (6) M debugger/src/gui.c (4) M debugger/src/plugin.c (4) M debugger/src/stree.c (4) M debugger/src/tpage.c (84)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/791.patch https://github.com/geany/geany-plugins/pull/791.diff
The maintainer of debugger is not very active lately, so anybody should feel free to review/test/comment otherwise this may be fairly slow to be added.
I'll try to have a look on it at the weekend.
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).
@bxgaillard pushed 1 commit.
5eec7fd Factorize some code between GTK2 and GTK3.
bxgaillard commented on this pull request.
@@ -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 agree. Fixed.
bxgaillard commented on this pull request.
calc_width = (gint) cell->xpad * 2 + pixbuf_width;
calc_height = (gint) cell->ypad * 2 + pixbuf_height; +#endif
I agree. Fixed.
bxgaillard commented on this pull request.
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;
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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
I agree. Fixed.
bxgaillard commented on this pull request.
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;
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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
I agree. I replaced the deprecated glib code with the new one.
bxgaillard commented on this pull request.
@@ -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 agree. I replaced the deprecated glib code with the new one.
bxgaillard commented on this pull request.
@@ -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
This change has been made to use the new `g_cond_wait_until()` function which replaces the deprecated `g_cond_timed_wait()` one. The old code has been replaced with the new one.
bxgaillard commented on this pull request.
}
+#if GTK_CHECK_VERSION(3, 0, 0) + while (!g_cond_wait_until(&cond, &change_config_mutex, interval)); + g_mutex_unlock(&change_config_mutex);
Fixed: the old code has been replaced with the new one.
bxgaillard commented on this pull request.
@@ -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
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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);
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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
I used the common utils library, thanks for the suggestion.
bxgaillard commented on this pull request.
@@ -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
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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;
Fixed: old code replaced with new one.
bxgaillard commented on this pull request.
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);
Maybe it could, but `gtk_box_set_homogeneous()` is used because of the missing parameter in `gtk_box_new()`. Using it in the GTK2 version would be redundant.
bxgaillard commented on this pull request.
@@ -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);
Same as above : it would be redundant in the GTK2 code.
bxgaillard commented on this pull request.
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();
I agree. Fixed.
bxgaillard commented on this pull request.
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);
I agree. Fixed.
bxgaillard commented on this pull request.
@@ -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));
I agree. Fixed.
Thank you @LarsGit223 for the thorough PR review! I'm not a GTK+ expert at all, that's why I didn'y dive in the documentation and just applied the changes from #645 with preprocessing conditionals. As indicated in my replies I've applied your suggestions and pushed a new commit which should be more adequate.
LarsGit223 commented on this pull request.
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);
Ok, then let's keep it as-is.
LarsGit223 commented on this pull request.
@@ -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);
Ok, let's keep it to.
LarsGit223 approved this pull request.
This looks much better now because of all the code that is now shared between gtk2 and 3. Also, the deprecation warnings I mentioned when compiling for gtk2 seem to be gone.
Compiled/installed/tested debugging shortly on gtk2 and 3 again, still works.
So I give it my OK.
LarsGit223 approved this pull request.
I'm not a GTK+ expert at all, that's why I didn'y dive in the documentation and just applied the changes from #645 with preprocessing conditionals.
Therefore thanks for putting time in it even though it wasn't your PR originally :+1: Once the others agree to merge it, the only thing left to do would be to squash the commits into one.
Therefore thanks for putting time in it even though it wasn't your PR originally :+1:
My pleasure.
Once the others agree to merge it, the only thing left to do would be to squash the commits into one.
Done.
@b4n @frlan are you going to wait for the maintainer or will you merge for 1.34?
I did work on the debugger plugin and am willing to look at this, but I won't have time for 1.34
Oh well, no debugger for Debian and descendants with 1.34. Note that @LarsGit223 reviewed and presumably tested and he is scope maintainer.
An IDE without a debugger, blocked in this state for the lifetime of the next stable Debian release, that would be very sad.
@elextr, @b4n: Yes, I reviewed this. I have to admit that I never wrote a debugger myself. But the things changed here only apply to the GUI, not to the debugger related internals.
I tested the following: - building under Ubuntu with GTK2 and 3 - simple test under Ubuntu with GTK2 and 3: run a program in the debugger, have some breakpoints. Inspect some variables. I clicked some icons and dialogs, nothing seems to be broken. - I did not test the GUI extensively (that means, I did **not** make a checkpoint list of all debugger related icons and menus and tested each of it) - I did not test or build under Windows or Mac - I had a look at the GTK API calls and the GTK versions for which they are supported and suggested changes (see the review for details) - Changes were applied and I re-tested and re-viewed again
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`?
elextr commented on this pull request.
- 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;
Whats the point of complicating things by making them "private"? This is a debugger plugin, not a library that needs to be protected from being used by millions.
b4n commented on this pull request.
- 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;
Yeah it probably doesn't matter much here indeed. I just mentioned that because the code previously used `GSEAL()` that's all.
bxgaillard commented on this pull request.
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're right, changed.
bxgaillard commented on this pull request.
cell_renderer->mode = GTK_CELL_RENDERER_MODE_ACTIVATABLE;
+#endif
Fixed.
bxgaillard commented on this pull request.
- 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;
Yes I didn't understand the reason to use GSEAL() in the first place in the GTK+2 code. It just broke the GTK+3 version. As a more elegant solution to the private nature of these members, I moved the structure definitions out of the .h into the .c.
bxgaillard commented on this pull request.
- GdkPixbuf *GSEAL (pixbuf_active); - GdkPixbuf *GSEAL (pixbuf_highlighted); + GdkPixbuf *pixbuf_active; + GdkPixbuf *pixbuf_highlighted;
Same answer as above.
bxgaillard commented on this pull request.
}
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);
The code can't be the same between GTK+2 and GTK+3 because the GtkTextView (and the GtkTreeView) don't implement the GtkScrollable interface in GTK+2 whereas they do in GTK+3 and shall seemingly be handled with a GtkScrolledWindow to add the scrolling functionality from what I can understand.
However you're absolutely right about the use before assignment, this is fixed.
bxgaillard commented on this pull request.
@@ -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 as above. And the correct variable is effectively tree, this is fixed.
bxgaillard commented on this pull request.
@@ -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);
If you're talking about the use of `gtk_widget_set_can_focus()`, that's right and it's fixed. The remaining code about PTY initialization seemingly has to be different though.
Thank you @b4n for your review. I applied your proposals, squashed the commits and pushed it.
b4n commented on this pull request.
}
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);
Be it in GTK2 or GTK3, to actually be scrollable you need to put most widgets in a GtkScrolledWindow. The widgets "supporting scrolling" like GtkTreeView and GtkTextView know how to communicate with the ScrolledWindow. On GTK3 it's a proper interface; on GTK2 it was signals only. Basically, what you usually would do is simply create a ScrolledWindow with passing `NULL` as the adjustments, and let GTK do it's thing when you add the child widget.
So, basically you could just use `gtk_scrolled_window_new(NULL, NULL)` followed by `gtk_container_add()` in bot GTK2 and GTK3.
b4n commented on this pull request.
@@ -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);
Yes I'm talking about `gtk_widget_set_can_focus()` I don't really know about the PTY stuff, but it seem that it has indeed to be different with VTE2.91.
bxgaillard commented on this pull request.
}
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);
Thank you for taking the time to explain this to me. I changed the corresponding code accordingly, as well as some other code which used the same construction for fewer differences between GTK+2 and GTK+3 in debug.c.
I can test this on Debian buster and OpenBSD 6.4. Let me know if that's needed.
@andy5995 testing is always appreciated
So far, mostly looks good on Buster (I set a breakpoint, watched a variable.
One very minor thing.. when debugging a program that uses ncurses, I was able to view the screen in the Debug terminal as expected. The cursor keys worked within the terminal fine.
But when I hit the green arrow (continue button to move past a break point), I have to click back in the terminal window each time, otherwise the cursor responds in the document editor window.
That probably won't be a problem for anyone; I figured it out pretty quick.
As for OpenBSD I get this
``` checking whether the linker understands -Wl,-z,defs... yes ./configure[15275]: test: 3: unexpected operator/operand checking for UTILSLIB... no configure: error: Plugin Debugger depends on utilslib VTE support which is not available ``` These are "vte" packages available to me atm with the package manager:
``` root:/root:11# pkg_info -Q vte libvterm-20170211 (installed) py-vte-0.28.2p14 vte-0.28.2p19 vte3-0.52.2 (installed) vteplugin-0.1p13 ```
The vte3 package was already installed the first time I tried. Then I installed libvterm but got the same result, which is what I posted above.
Hi @andy5995, The problem you mention is with utilslib, which is part of geany-plugins and now used by the debugger plugin (it contains some compatibility functions). Did you activate it at configure time? It may need some other dependencies besides libvte.
I noticed the line ```...PTY_LIBS="-lutil"]``` in ```scope.m4```. Do we need this in ```debugger.m4``` also?
Hi @andy5995, The problem you mention is with utilslib, which is part of geany-plugins and now used by the debugger plugin (it contains some compatibility functions). Did you activate it at configure time?
[config.log](https://github.com/geany/geany-plugins/files/2788940/config.log)
I only tried activating it after you told me about it ;) But I got the same result. But now I wonder if the test failing is related to the error mentioned on L15275.
``` checking whether the linker understands -Wl,-z,defs... yes ./configure[15275]: test: 3: unexpected operator/operand checking for UTILSLIB... no configure: error: Plugin Debugger depends on utilslib VTE support ```
I looked but couldn't find the problem.
I installed the vte-0.28.2p19 package and that problem was solved. New problem is that the built fails with an undefined reference error:
https://gist.github.com/andy5995/0b0fab05c70336bfff86ac4a3354d5a6#file-gistf...
[config.log](https://github.com/geany/geany-plugins/files/2789099/config.log)
Could be related to @LarsGit223 's comment, as I see that function seems to be defined in the utility library.
I tried preceding the configure command with PTY_LIBS="-lutil" and LDADD"-lutil" but no effect.
`./configure[15275]: test: 3: unexpected operator/operand`
Could you paste your generated `configure`? The error probably comes from there and a `test` call that doesn't please your shell
`./configure[15275]: test: 3: unexpected operator/operand`
Could you paste your generated `configure`? The error probably comes from there and a `test` call that doesn't please your shell
Sure...
https://gist.github.com/andy5995/fb2b16eb9dab45d9ab8160cdd6f2a48e
Also noticed it happening with other tests as well
``` checking libutil.h presence... no checking for libutil.h... no checking for glib-genmarshal... /usr/local/bin/glib-genmarshal checking for glib-mkenums... /usr/local/bin/glib-mkenums checking for rst2html... /usr/local/bin/rst2html ./configure[18255]: test: 3: unexpected operator/operand checking for GTKSPELL... no checking for gpgme-config... no checking for special C compiler options needed for large files... no checking for _FILE_OFFSET_BITS value needed for large files... no ./configure[19681]: test: 3: unexpected operator/operand ./configure[21201]: test: 3: unexpected operator/operand checking for util.h... (cached) yes checking for pty.h... (cached) no checking for libutil.h... (cached) no checking for ENCHANT_2_2... no checking for ENCHANT_1_5... no checking for ENCHANT_2_0... no checking for glib-mkenums... (cached) /usr/local/bin/glib-mkenums ./configure[22250]: test: 3: unexpected operator/operand ```
Apparently we've got a portability issue with the GTK version check. @andy5995 could you check out #815 and see if it helps? It should get rid of the *"unexpected operator/operand"* issues, and hopefully maybe more.
#815 fixed the build issue and configure warnings on OpenBSD.
One problem I noticed on Debian Buster and Slackware-current: when the Debugger plug-in is enabled, the "down arrow" disappears (screenshot below).
My debug info from Buster:
``` 15:36:41: Geany INFO : Using alternate configuration directory 15:36:41: Geany INFO : Geany 1.35 (git >= bf5c9edd), en_US.utf8 15:36:41: Geany INFO : GTK 3.24.3, GLib 2.58.2 15:36:41: Geany INFO : System data dir: /home/andy/test-geany/share/geany 15:36:41: Geany INFO : User config dir: /home/andy/test-geany/config 15:36:41: Geany INFO : Loaded GTK+ CSS theme '/home/andy/test-geany/share/geany/geany.css' 15:36:41: Geany INFO : Loaded GTK+ CSS theme '/home/andy/test-geany/share/geany/geany-3.20.css' 15:36:41: Geany INFO : System plugin path: /home/andy/test-geany/lib/geany 15:36:41: Geany INFO : Added filetype Clojure (61). 15:36:41: Geany INFO : Added filetype Graphviz (62). 15:36:41: Geany INFO : Added filetype CUDA (63). 15:36:41: Geany INFO : Added filetype JSON (64). 15:36:41: Geany INFO : Added filetype Arduino (65). 15:36:41: Geany INFO : Added filetype Scala (66). 15:36:41: Geany INFO : Added filetype Genie (67). 15:36:41: Geany INFO : Added filetype Cython (68). 15:36:41: Geany INFO : Loaded libvte from libvte-2.91.so 15:36:41: Geany INFO : Loaded: /home/andy/test-geany/lib/geany/debugger.so (Debugger) 15:36:41: Geany INFO : unknown : None (UTF-8) 15:36:49: GdkPixbuf DEBUG : gdk_pixbuf_from_pixdata() called on: 15:36:49: GdkPixbuf DEBUG : Encoding raw 15:36:49: GdkPixbuf DEBUG : Dimensions: 14 x 14 15:36:49: GdkPixbuf DEBUG : Rowstride: 56, Length: 808 15:36:49: GdkPixbuf DEBUG : Copy pixels == false 15:36:49: GdkPixbuf DEBUG : gdk_pixbuf_from_pixdata() called on: 15:36:49: GdkPixbuf DEBUG : Encoding raw 15:36:49: GdkPixbuf DEBUG : Dimensions: 14 x 14 15:36:49: GdkPixbuf DEBUG : Rowstride: 56, Length: 808 15:36:49: GdkPixbuf DEBUG : Copy pixels == false 15:36:49: GdkPixbuf DEBUG : gdk_pixbuf_from_pixdata() called on: 15:36:49: GdkPixbuf DEBUG : Encoding raw 15:36:49: GdkPixbuf DEBUG : Dimensions: 14 x 14 15:36:49: GdkPixbuf DEBUG : Rowstride: 56, Length: 808 15:36:49: GdkPixbuf DEBUG : Copy pixels == false ```
One problem I noticed on Debian Buster and Slackware-current: when the Debugger plug-in is enabled, the "down arrow" disappears
Does that reproduce on Ubuntu as well? @LarsGit223 ?
@andy5995: Yes it does.
Tested with your repository branch ```debugger-gtk3```: - Ubuntu 18.04.02, gtk 3.22.30, glib 2.56.3: the arrow is missing - Ubuntu 18.04.02, gtk 2.24.32, glib 2.56.3: the arrow is showing
Just in case you did not notice because "the down arrow is missing" does not describe the problem I noticed very good. I would say enabling the ```debugger``` plugin on gtk3 causes the menu bar in the message window to have a fixed size. It is not shrinkable/growable any more. So the arrows are gone. Also the menu items are not reduced any more if the height of the message window is too small (I mean that usually e.g. only two menu items are shown if there is only space for two and then you can use the arrows to reach the other buttons). Disabling the ```debugger``` plugin immediately brings back the usual behavior.
I had another look at the changes made by the PR but I cannot find the reason why the re-sizing of the message window does not work as expected under gtk3 if the debugger plugin is enabled.
I've looked into it and from what I can tell this is normal behaviour.
The main Geany GUI uses GtkPaneds for side panels (document browser, symbols, messages...), including the bottom one. When such a panel is too small to display its contents, the latter is located in hidden overflow.
This also happens for the left panel if you resize it: try to drag the splitter to the left, you'll see that the left arrow will slide under the left border of the main window. The exact same thing happens with the debugger plugin if the botton panel becomes too small. Or put more exactly: too small for the right button panel to fit un the bottom space.
So IMHO this "bug" is unrelated to the debugger plugin but is a general Geany GUI issue.
I can see two possible resolutions: 1. Fix the main Geany GUI so that the side panels don't overflow either by enabling an automatic scrollbar or by specifing a minimum size the panels can't be shrink under (which I think is the best one); 2. Add a scrollbar specific to the whole debugger plugin panel, with possible ergonomic issues.
I also checked the GTK+2 behaviour : it simply doesn't honor the plugins minimum panel size. When you shrink the panel so that it becames too small, the controls collapse one above another and the GUI isn't quite usable anymore. This isn't better IMHO.
BTW, on an unrelated note, I'm going to rebase the branch on more current code. No other change beyond that.
@bxgaillard have you tried setting sizes and resize and shrink and stuff from CSS, setting minimum sizes in apps is not very portable. Note also the comment from @LarsGit223 that showed the behaviour was different on different versions of gtk. which did you test on?
@bxgaillard have you tried setting sizes and resize and shrink and stuff from CSS, setting minimum sizes in apps is not very portable.
I have made some research and CSS seems not to be appropriate for setting dimensions. On the other hand the correct way to fix it seems to be the Glade file. Changing <property name="shrink">True</property> to False (Geany release 1.34.1, geany.glade line 8452) doesn't allow overflow for the bottom paned for instance. But as I said just before this has to be fixed in the main Geany repository and has nothing to do with geany-plugins.
Note also the comment from @LarsGit223 that showed the behaviour was different on different versions of gtk. which did you test on?
I tested on both: with GTK+3 the hidden overflow happens and with GTK+2 the controls collapse (they superpose themselves) and the right button panel from the debugger plugin overflows (instead of the whole bottom panel).
@elextr, @bxgaillard: if the behavior is really caused by a different behavior of gtk2 and gtk3 then it is OK for me. As I wrote I couldn't find anything in the code between the two versions which looked like a porting fault. Also, if someone enables the debugger plugin it doesn't really make sense to make the message window that small that you recognize this issue/difference in behavior.
This didn't make it into 1.35 but maybe we can put it to milestone 1.36. As written earlier the little overflow problem is acceptable to me.
Well...let's go forward.
Merged #791 into master.
As someone who enjoys using Geany, but is quite new to Linux in general, how would I be able to use this debugger on my machine? I have noted that I cannot use it, and tracked down the issue to here, but am unsure as to how I would use this, or even where to download these plugins from. Thanks for your time, and sorry for any inconvenience.
When you post to github or mailing lists (which is where you should ask for help, github is for software improvements and bug reports) you should always provide more than "Linux" which is meaningless, you need to provide your distribution and version and the version of Geany and GTK it uses (see near the top of help debug messages).
My apologies, I am on an ubuntu 18.04 system, and noticed that the debugger was not on my plugin list, and through some digging found it was due to having GTK3, rather than GTK2. I want to know how I would download the debugger that is in this repo, and use it on my machine. I am also using Geany 1.36.
To repeat "the version of Geany and GTK it uses (see near the top of help debug messages)." Just copy and paste the information from there.
When you are asked for information you need to answer the question or grumpy olde sods like me will tell you off :grin: and as I said above, if its a question do it on the mailing list and also do NOT hijack existing issues for your personal questions.
@AnishGRao see #942
github-comments@lists.geany.org