Tested on Fedora 27 using stock Geany available from repo. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/645
-- Commit Summary --
* Ported 'Debugger' plugin to GTK+3.
-- File Changes --
M build/debugger.m4 (4) M debugger/src/bptree.c (6) M debugger/src/btnpanel.c (15) M debugger/src/cell_renderers/cellrendererbreakicon.c (66) M debugger/src/cell_renderers/cellrendererbreakicon.h (14) M debugger/src/cell_renderers/cellrendererframeicon.c (57) M debugger/src/cell_renderers/cellrendererframeicon.h (6) M debugger/src/cell_renderers/cellrenderertoggle.c (9) M debugger/src/dconfig.c (47) M debugger/src/debug.c (43) M debugger/src/dpaned.c (5) M debugger/src/gui.c (2) M debugger/src/plugin.c (2) M debugger/src/stree.c (2) M debugger/src/tpage.c (62)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/645.patch https://github.com/geany/geany-plugins/pull/645.diff
@cesspit Can you please review?
b4n commented on this pull request.
cell->enabled = TRUE; cell->condition = NULL; cell->hitscount = 0; - - cell_renderer->mode = GTK_CELL_RENDERER_MODE_ACTIVATABLE; + + g_value_init(&mode, G_TYPE_ENUM); + g_value_set_enum(&mode, GTK_CELL_RENDERER_MODE_ACTIVATABLE); + g_object_set_property(G_OBJECT(cell_renderer), "mode", &mode); + g_value_unset(&mode);
what about `g_object_set(cell_renderer, "mode", GTK_CELL_RENDERER_MODE_ACTIVATABLE, NULL);`? simpler and virtually identical.
b4n commented on this pull request.
@@ -1,9 +1,9 @@
AC_DEFUN([GP_CHECK_DEBUGGER], [ GP_ARG_DISABLE([Debugger], [auto]) - GP_CHECK_PLUGIN_GTK2_ONLY([Debugger]) + GP_CHECK_PLUGIN_GTK3_ONLY([Debugger])
I didn't check how hard it'd be to support both GTK2 and GTK3, but it seems a little sad to switch from one restriction to the other. Well, I guess it becomes less and less mandatory to support GTK2, but still.
I didn't check how hard it'd be to support both GTK2 and GTK3, but it seems a little sad to switch from one restriction to the other. Well, I guess it becomes less and less mandatory to support GTK2, but still.
@b4n, @pmjobin suggest the same thing I have said elsewhere, don't support both in one plugin just make this GTK3 version a separate plugin, named "debugger-gtk3" for eg, the build protections should prevent both being built, and loading protections won't let the wrong one be loaded into Geany if somehow both get built.
Then all that has to happen later is the GTK2 version just disappears.
@elextr I don't really see this as a valid approach to support both, more of a way of not having the gut of saying GTK2 version is gone (no offense, I just didn't find a nice way to put it). If really the other one is dead, just say that if somebody wants the GTK2 version they oughta use the one from GP vX.YY, which is older but that's life. I feel like having both is meant to lead to duplication of effort, or worse having some people contributing to the GTK2 and some to the GTK3 version.
Anyway, it's just my 2¢ and it's up to the maintainer of the plugin (which, even if I fixed a bunch of big issues I'm not), and for myself I don't really care as I use GTK3 but for testing purposes anyway.
I feel like having both is meant to lead to duplication of effort, or worse having some people contributing to the GTK2 and some to the GTK3 version.
@b4n, yes I would agree totally, if there was active development going on, but last commit on debugger is December __2016__.
Anyway, it's just my 2¢ and it's up to the maintainer of the plugin (which, even if I fixed a bunch of big issues I'm not), and for myself I don't really care as I use GTK3 but for testing purposes anyway.
A new plugin doesn't need the permission of the maintainer of the GTK2 version, so if @pmjobin submitted a PR for a whole plugin it would be up to @frlan `</ shifting responsibility, management 101>` :grin:.
@elextr myeah maybe as it's not really maintained it could make some sense. Yet, from past experience only a few things are tricky to keep compatible between both versions, so (without having looked carefully) I wouldn't think most of is would be hard to keep compatible. And removing deprecation is fun and all, but might only be useful for GTK4+, which might require a lot more changes anyway.
As for the permission, as clever as it is the plugin is unmaintained (no matter what the MAINTAINERS file states, Alexander is gone as of the latest news I had) anyway, so it's up for grabs :)
As for the permission, as clever as it is the plugin is unmaintained (no matter what the MAINTAINERS file states, Alexander is gone as of the latest news I had) anyway, so it's up for grabs :)
Wasn't aware of that, so yeah, its up to the contributor.
As for GTK4, thats a whole other kettle of poison `</deliberate misspelling of poisson>`, but until Geany looks at it, its not worth thinking about for an unmaintained plugin :)
mattcaswell requested changes on this pull request.
cell->enabled = TRUE; cell->condition = NULL; cell->hitscount = 0; - - cell_renderer->mode = GTK_CELL_RENDERER_MODE_ACTIVATABLE; + + g_value_init(&mode, G_TYPE_ENUM);
This should be:
g_value_init(&mode, GTK_TYPE_CELL_RENDERER_MODE);
Otherwise you get errors like this:
```` (geany:16953): GLib-GObject-WARNING **: 00:27:20.736: ../../../../gobject/gvalue.c:188: cannot initialize GValue with type 'GEnum', this type is abstract with regards to GValue use, use a more specific (derived) type
(geany:16953): GLib-GObject-CRITICAL **: 00:27:20.736: g_value_set_enum: assertion 'G_VALUE_HOLDS_ENUM (value)' failed
(geany:16953): GLib-GObject-CRITICAL **: 00:27:20.736: g_value_transform: assertion 'G_IS_VALUE (src_value)' failed
(geany:16953): GLib-GObject-WARNING **: 00:27:20.736: unable to set property 'mode' of type 'GtkCellRendererMode' from value of type '(null)' ````
@@ -53,7 +53,12 @@ static gint cell_renderer_toggle_activate(GtkCellRenderer *cell, GdkEvent *event
static void cell_renderer_toggle_init (CellRendererToggle *cell) { GtkCellRenderer *cell_renderer = (GtkCellRenderer*)cell; - cell_renderer->mode = GTK_CELL_RENDERER_MODE_ACTIVATABLE; + GValue mode = G_VALUE_INIT; + + g_value_init(&mode, G_TYPE_ENUM);
Again this should be:
g_value_init(&mode, GTK_TYPE_CELL_RENDERER_MODE);
cell->pixbuf_active = cell->pixbuf_highlighted = 0;
+ + g_value_init(&mode, G_TYPE_ENUM);
Again this should be:
g_value_init(&mode, GTK_TYPE_CELL_RENDERER_MODE);
I found this through testing this PR. Without doing the above you cannot change the current active frame in the stack.
@pmjobin pushed 1 commit.
04f58bb Simplified syntax for initialization of debugger plugin icons.
@mattcaswell Thanks for your help. I pushed a fix to simplify handling of GTK_CELL_RENDERER_MODE based on an earlier suggestion by @b4n. Changing the current active frame in the stack seems to work as expected with this fix applied.
Thanks @pmjobin. I tried this out, and I can confirm that the updated code runs without errors, and I am able to change the active frame in the stack.
This pull-request works for me also.
with multiple people having confirmed that this works, is there any reason why this is currently staled?
Because it only supports GTK3 and no longer supports GTK2.
@frlan @b4n given Fedora and Debian and all its derivatives now distribute a GTK3 version of Geany thats the majority of users. There are not many left distributing the GTK2 versions of Geany. You are gonna have to decide at some point to give up on trying to force plugin maintainers to support GTK2 as well by refusing to commit GTK3 only ports.
Just to be clear, the policy of refusing GTK3 only changes is now meaning most new or updating users have no debugger with Geany.
From a glance, it appears the only thing making this GTK3-only is the changes to the build system which require GTK3 instead of either major version.
FYI I created a new pull request that doesn't break compatibility with GTK2: #790.
This PR is still on the 1.34.0 milestone and should maybe be replaced with PR #791.
Closed in favour of #791
Closed #645.
github-comments@lists.geany.org