Skip to content
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

Support overrides #10

Merged
merged 6 commits into from
Jan 21, 2024
Merged

Support overrides #10

merged 6 commits into from
Jan 21, 2024

Conversation

vixalien
Copy link
Collaborator

also move gobject.object overrides to new file
allowing to construct objects by extending the existing object

@vixalien
Copy link
Collaborator Author

This pattern is similar to what GJS does already, for example: https://gitlab.gnome.org/GNOME/gjs/-/blob/HEAD/modules/core/overrides/GLib.js

@vixalien
Copy link
Collaborator Author

This PR also needs to get merged later, converting to draft.

@vixalien vixalien marked this pull request as draft December 16, 2023 20:14
@vixalien vixalien mentioned this pull request Dec 17, 2023
@vixalien vixalien force-pushed the overrides branch 3 times, most recently from 5378d5e to 911db9d Compare December 23, 2023 00:20
@vixalien vixalien mentioned this pull request Dec 26, 2023
2 tasks
@vixalien vixalien force-pushed the overrides branch 2 times, most recently from 5e34466 to e40a224 Compare January 6, 2024 14:18
@vixalien vixalien force-pushed the overrides branch 3 times, most recently from 1cb3437 to a4eddcc Compare January 15, 2024 17:39
@vixalien vixalien marked this pull request as ready for review January 15, 2024 17:42
@vixalien
Copy link
Collaborator Author

@ahgilak, I believe this is ready for review. I would like to know what you feel about this. This will be the basis of other very cool stuff I'm working on, such as #23 and #20

@ahgilak
Copy link
Owner

ahgilak commented Jan 15, 2024

Are you sure this is ready?
Everything seems broken.

also move gobject.object overrides to new file
allowing to construct objects by extending

remove girepository symbol
@vixalien
Copy link
Collaborator Author

Hmm. I see the examples are not working. However, they are not working in the main branch either, so I think it's a regression I caused. I'm really sorry for this, and think it's time we added some tests.

…aded

previously, doing `require("Adw", "1")` would fail to register `GObject.Object.connect`, as the `GObject` namespace would not be loaded. Currently, now `require` will also load the given dependencies of the loaded namespace, so that their overrides can be loaded.
@vixalien
Copy link
Collaborator Author

Wait. I see what happened now. I got errors while trying to execute the files in the examples/ folder, and it turns out they were using an older version of deno_gi because they got it from https://gir.deno.dev. Could you add versioning to the domain like it's done for https://deno.land/x? It may even be a good idea to make the site's code open source if possible.

This PR is ready for re-review, as it works, please let me know what you think.

@vixalien vixalien mentioned this pull request Jan 17, 2024
36 tasks
@ahgilak
Copy link
Owner

ahgilak commented Jan 18, 2024

Hmm. I see the examples are not working. However, they are not working in the main branch either

Only stylesheet.ts is broken on main, other examples are fine.

@ahgilak
Copy link
Owner

ahgilak commented Jan 18, 2024

main purpose of gir.deno.dev is to add typescript definitions, besides that it only loads the library and nothing else.

you can test your local changes by adding the following deno.json file:

{
  "imports": {
    "https://github.com/ahgilak/deno_gi/raw/main/mod.ts": "./mod.ts"
  }
}

@ahgilak
Copy link
Owner

ahgilak commented Jan 18, 2024

This is the error comes up while running the examples in current PR

error: Uncaught (in promise) TypeError: app.on is not a function
app.on("activate", activate);
    ^

@vixalien
Copy link
Collaborator Author

main purpose of gir.deno.dev is to add typescript definitions, besides that it only loads the library and nothing else.

Yes, I see. it can still get versioned right?

Only stylesheet.ts is broken on main, other examples are fine.

The fixes for that are in #26. Do you want me to include that PR in this one? I wanted to include it in a separate PR because it fixes more things than this specific issue.

@ahgilak
Copy link
Owner

ahgilak commented Jan 19, 2024

Do you want me to include that PR in this one?

No I meant other examples are working fine on main branch, but broken in this PR.

@vixalien
Copy link
Collaborator Author

No I meant other examples are working fine on main branch, but broken in this PR.

Yes, I know :)

I was saying that that problem is indeed exposed by this PR, but the fix is in a separate PR because it handles other cases too of this bug. I will include the fix in this PR though, to streamline things.

this method gets the closest parent gtype that is registered in gobject-introspection. this is because classes registed manually (in deno_gi) can't be constructed from their GBaseInfo. In the future, we will probably need to maintain a reference to the manually registed classes so they can be constructed from reference.
instead of soft crashing, this method instead bails immediately
@vixalien
Copy link
Collaborator Author

fixed everything. please take a look!

@vixalien vixalien requested a review from ahgilak January 20, 2024 02:33
@ahgilak ahgilak merged commit 5e0aa29 into ahgilak:main Jan 21, 2024
@vixalien
Copy link
Collaborator Author

Thanks!

@vixalien vixalien deleted the overrides branch January 21, 2024 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants