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

[doc update]: Module:init from wx_object behaviour should return wxWindow (and not … #7664

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

jurmc
Copy link
Contributor

@jurmc jurmc commented Sep 19, 2023

…wxObject which is not descendant of wxWindow)

I tried return sizer from Module:init (callback for wx_object module) and such initialization fails.
Current documentation is ambiguous, here https://github.com/erlang/otp/blob/master/lib/wx/src/wx_object.erl#L39 it states that wxObject can be returned from init, but ultimately start/start_link functions are documented so they return wxWindow (for example here: https://github.com/erlang/otp/blob/master/lib/wx/src/wx_object.erl#L197

Implementation definitely fail if Module:init returns something different than wxWindow, see here
https://github.com/erlang/otp/blob/master/lib/wx/src/wx_object.erl#L421

@github-actions
Copy link
Contributor

github-actions bot commented Sep 19, 2023

CT Test Results

    2 files    17 suites   5m 37s ⏱️
155 tests 150 ✔️ 5 💤 0
171 runs  166 ✔️ 5 💤 0

Results for commit 0b64f36.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@jurmc jurmc changed the title Module:init from wx_object behaviour should return wxWindow (and not … [doc update]: Module:init from wx_object behaviour should return wxWindow (and not … Sep 21, 2023
@dgud
Copy link
Contributor

dgud commented Sep 21, 2023

You need to update lib/wx/doc/src/wx_object.xml
The internal edoc is currently not used.

@dgud dgud self-assigned this Sep 21, 2023
@jurmc
Copy link
Contributor Author

jurmc commented Sep 21, 2023

You need to update lib/wx/doc/src/wx_object.xml The internal edoc is currently not used.

Thanks, done: 97ef96b

@jurmc
Copy link
Contributor Author

jurmc commented Sep 21, 2023

@dgud I could try to relax restrictions so maybe 'instances' of Sizers could be objects that are legal for wxObject behaviour. I'm asking if you are aware of anything in the design or implementation that renders this impossible to do or unwanted?

Of course I accept, that even if I propose something that works for me, it will be rejected for whatever reasons you'll then have.

@dgud
Copy link
Contributor

dgud commented Sep 21, 2023

I don't see the need, to me wx_object was something that could handle events (and event callbacks) and maybe add some erlang only extensions.

Adding true overloading to C++ functionality is a lot of work/hacks, that I want to avoid as far as possible.

What are you trying to do?

@dgud
Copy link
Contributor

dgud commented Sep 21, 2023

Please squash the commits and force push.

…wxObject which is not descendant of wxWindow)
@jurmc
Copy link
Contributor Author

jurmc commented Sep 21, 2023

Please squash the commits and force push.

Done

@dgud dgud added the testing currently being tested, tag is used by OTP internal CI label Sep 21, 2023
@dgud dgud merged commit 7a511c5 into erlang:master Sep 26, 2023
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants