-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Box hierarchy for GTK4 #313
Conversation
I have no clue about this, but I have the powers to merge this. Is there anyone with some clue wanting to chime in? Also, should "something" be added to the test suite for this? (I don't know what / how to test this; I am just thinking out loud.) |
lgi/override/Gtk.lua
Outdated
@@ -219,6 +219,9 @@ if Gtk.Container then | |||
end | |||
end | |||
|
|||
-------------------------------- Gtk.Box overrides. | |||
Gtk.Box._container_add = Gtk.Box.append |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to wrap this with a GTK version check to only apply to GTK >= 4
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. This method doesn't exist in GTK3. I'll look into moving this down the bottom the file inside a "GTK4 overrides" block, right next to the old GTK2 check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as of the latest commit:
Gtk override
-------------------------------- Gtk-4 overrides
if Gtk._version == '4.0' then
--- CONTAINER CLASSES
--- GTK4 Has removed the Container abstract class.
--- This means Boxes, Grids and other layout widgets
--- Must now add children using their own specific methods.
--- Gtk.Box overrides
--- widgets on a the array part of the constructor of a Box
--- will be `append()`-ed
--- in order of appearance.
Gtk.Box._container_add = Gtk.Box.append
end
This will prevent Gtk versions ~= 4.0 from attempting Gtk.Box.append()
.
Container override substitute for GTK4It might be possible to reuse the same logic from the Container class override to bring nested widget constructors back without duplicating the whole thing for each class. If i get it working, i'll make another PR. |
I think we can take a look at how other Language Bindings are doing, https://www.gtk.org/docs/language-bindings/index. GJS and PyGOject seems to be good references: https://gitlab.gnome.org/GNOME/gjs/-/blob/master/modules/core/overrides/Gtk.js https://gitlab.gnome.org/GNOME/pygobject/-/blob/master/gi/overrides/Gtk.py According to https://gitlab.gnome.org/GNOME/pygobject/-/merge_requests/148 and https://gitlab.gnome.org/GNOME/pygobject/-/merge_requests/150, PyGObject took the decision to keep a "phantom class" for Container (I'm not sure to correctly understand what the second MR changed in this regard). GJS seems to use JavaScript prototype magic 🤷 https://gitlab.gnome.org/GNOME/gjs/-/commit/bdb66678e0b09802747767e40f66208c2a02e359 and https://gitlab.gnome.org/GNOME/gjs/-/commit/2ede057f18c0c6a22b98fe04118ff06304b4a1cb |
So, just so we're on the same page here... is it OK to merge? are we missing something? does it need a corresponding "container methods" test? are we waiting on more reviewers? it this GTK4 progress snippet not enough to warrant a PR? Should i push it to a "GTK-4 WIP" branch instead? |
No worry's, I did some investigation and and have commented my findings. The "container thing" can be investigated and sorted out with other PRs. It will probably require a lot of work. Regarding tests, I'm not sure how to proceed, and we probably don't have the necessary tooling right now. I think we can merge. (I can't do it tho) |
It works like this: The original maintainer disappeared and since then, things basically do not work. 🤷 |
What is this supposed to fix?
Running the following code should result in a window with a button with a box, but only the window shows.
Snippet: GTK4 window with a Box
Using the GTK4 interactive debugger with
<CTRL>+<SHIFT>+I
, you can browse through the widget hierarchy, where the ApplicationWindow object will have a Box child, but the Box will not have anything in it.Running the code with this fork's LGI will result in the hierarchy properly rendering, with a labeled button that says 'slap'.
How was this fixed?
Just a single line on the GTK override file, where i set the underlying
_container_add
gobject method of the Box type toBox.append()
.Are you seriously gonna publish just this naiive change to the main fork?
This pull request is for testing the waters. It is likely that more underlying code must be overriden for id navigation (e.g.
handles.atop.daBox.daButton.label='press me'
), child-on-child, other Container-like widgets that are no longer containers as of Gtk4, etc.Is there demand for this library with Gtk4? are people gonna use lua constructors instead of the language-independent builder XML format? what kind of knowledge on lgi's internals do i need to accomodate for GTK4's API breakage? I would very much like a little catching-up on this matter.