Navigation Menu

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

Update documentation to reflect recent Bind() changes #362

Closed
wants to merge 1 commit into from
Closed

Update documentation to reflect recent Bind() changes #362

wants to merge 1 commit into from

Conversation

tambry
Copy link
Contributor

@tambry tambry commented Dec 3, 2016

Adds the documentation changes from #17623, that weren't merged initially due to a couple problems.

Copy link
Contributor

@vadz vadz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry but I don't understand why are you resubmitting exactly the same changes that I already said I didn't want to apply because they didn't seem neither clear nor even correct to me.

To repeat for one last time, I didn't apply these changes intentionally, not because I forgot about them, but because they just seem wrong.

@@ -242,7 +242,7 @@ Now let us describe the semantic differences:

<li>
As a slight extension of the above, the handlers can also be unbound at
any time with Unbind<>() (and maybe rebound later). Of course,
any time with Unbind<>() (and may be rebound later). Of course,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changes the meaning slightly. Originally it was meant to say that the handlers could, perhaps, be re-bound later if needed. I'm not sure if the new wording really makes it more clear.

@@ -349,6 +349,33 @@ MyFrame::MyFrame()
}
@endcode

You can also bind functions that are members of any class provided that the
class publicly derives from wxTrackable from somewhere down the line and that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part seems just wrong to me, you can bind methods of any class, full stop. Why does it have to derive from wxTrackable?

@endcode

You can also bind functions for classes that don't publicly derive from wxTrackable
somewhere down the line, provided that your compiler supports C++11.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this seems wrong to me (and very confusing even if it were true, you can't say "you can only do this if that" only to add later "but you can also do this even if not that, sometimes").


You can also bind functions for classes that don't publicly derive from wxTrackable
somewhere down the line, provided that your compiler supports C++11.
Do note that in such cases event propagation will @b not work).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is absolutely not clear and, to repeat for the N-th time, I'm not sure if it's correct neither. What exactly does this mean? What exactly doesn't work?

@tambry
Copy link
Contributor Author

tambry commented Dec 4, 2016

I only addressed the only point you brought up in the ticket - the wording about the event chain being wrong, which I changed to reflect what you though would be right in the ticket's comment:47.

You brought up quite a few issues with the patch, this may be better to be addressed by someone, that knows more about wxWidgets' internals.

@tambry tambry closed this Dec 4, 2016
@tambry tambry deleted the bind_doc_fix branch June 30, 2018 16:05
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.

None yet

2 participants