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

Browser: Port browser to GML compilation #25399

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

overriden-sfdd
Copy link
Contributor

This PR contributes to #20518
It ports Browser. I'm open for suggestions regarding refactoring of BrowserTabWidget. It's quite puzzling. I'm not sure the way I did it is the best. Please review carefully.
Additionally, I got confused with all the different initialization functions like try_create, initialize, and setup. Do we prefer initialize over setup now?

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Nov 15, 2024
@overriden-sfdd
Copy link
Contributor Author

Oh yes, one more thing to add that felt kinda wrong. The GML compiler works in a way that prevents forward declaration in widget headers. I wonder if there's anything we can do about that and if that's intended. Widget headers become quite huge because of that.

@BuggieBot
Copy link
Member

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

@nico
Copy link
Contributor

nico commented Nov 28, 2024

Looks like this needs rebasing now, sorry!

As for the questions, iirc @kleinesfilmroellchen wrote the GML compiler. Film, do you know what we do about forward declarations? Just not worry about it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 pr-needs-review PR needs review from a maintainer or community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants