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

Missing function and method definitions in Example #1 for register_tick_function() #4420

Merged
merged 3 commits into from
Feb 7, 2025

Conversation

UsingSession
Copy link
Contributor

The example for register_tick_function() is incomplete. It is missing the definitions for the callback function my_function and the class method my_method.

Additionally, due to the missing definitions, the "Sandbox Example" code provided on the PHP documentation page cannot be run directly, making it difficult for users to test or understand the functionality.

Missing function and method definitions in Example #1 for register_tick_function()

@Girgias
Copy link
Member

Girgias commented Jan 30, 2025

I think the whole example should probably reworked at the same time, I don't really see the point in showcasing the different ways to pass a callable as that would also require examples with closures.

But yes, many examples cannot be executed as the WASM runner is new and many examples predate it by years/decades.

@UsingSession
Copy link
Contributor Author

@Girgias
I agree that the entire example can be reviewed and improved.

My main idea was to make the current code functional without major changes.

I accidentally came across this function on Stack Overflow and decided to learn more on the documentation site. Seeing the possibility to run the code, I decided to test it, but encountered an error, so ... yeah🙂

@Girgias
Copy link
Member

Girgias commented Feb 6, 2025

The ability to run code is only a recent feature to the website and many example do not work.

I would prefer to remove the various ways to set a callable (as that is general PHP behaviour rather than a register tick one) and just have a single function, which is removing code from the example to make it functional.

@UsingSession
Copy link
Contributor Author

UsingSession commented Feb 6, 2025

@Girgias

Sorry, but I made a mistake. I accidentally did a merge. Should I create a new PR?

@Girgias
Copy link
Member

Girgias commented Feb 7, 2025

I don't see the mistake? The PR seems fine.

@Girgias Girgias merged commit 689cf3b into php:master Feb 7, 2025
2 checks passed
@Girgias
Copy link
Member

Girgias commented Feb 7, 2025

Thank you!

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