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

Make sure ->app always holds a (weak) reference to the app #8

Merged
merged 1 commit into from Aug 10, 2017

Conversation

Projects
None yet
2 participants
@jjatria

jjatria commented Aug 10, 2017

App introspection is possible by holding a reference to the app in each
of its commands. In the past, this was the package name of the app's
package, but this made it impossible to have eg. multiple instances of
an app, or to have an app whose state is not entirely known at compile
time.

This patch solves this by holding a reference instead, which is made
weak to reduce the risk of memory leaks. For consistency (and since
a common pattern seems to be for apps to subclass both App::CLI
and App::CLI::Command), the app also gets a weak reference to itself.

An additional change in cmd_map makes sure the package name is properly
constructed.

Make sure ->app always holds a (weak) reference to the app
App introspection is possible by holding a reference to the app in each
of its commands. In the past, this was the package name of the app's
package, but this made it impossible to have eg. multiple instances of
an app, or to have an app whose state is not entirely known at compile
time.

This patch solves this by holding a reference instead, which is made
weak to reduce the risk of memory leaks. For consistency (and since
a common pattern seems to be for apps to subclass both App::CLI
and App::CLI::Command), the app also gets a weak reference to itself.

An additional change in cmd_map makes sure the package name is properly
constructed.

@paultcochrane paultcochrane merged commit 348b119 into paultcochrane:master Aug 10, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@paultcochrane

This comment has been minimized.

Show comment
Hide comment
@paultcochrane

paultcochrane Aug 10, 2017

Owner

Awesome, thanks!

Owner

paultcochrane commented Aug 10, 2017

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment