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

Inject spinner instance as parameter to decorated function #137

Closed
wants to merge 1 commit into from
Closed

Inject spinner instance as parameter to decorated function #137

wants to merge 1 commit into from

Conversation

romnn
Copy link

@romnn romnn commented Dec 30, 2019

I propose to inject the spinner instance as an optional parameter to functions decorated by halo. This was my use case:

@Halo(text='Doing work', spinner='dots', color="red")
def initialize_git(spinner=None):  # This argument is optional and can also be omitted
    try:
        my_complicated_function()
    except Exception:
        spinner.fail("Failed!")
        raise
    spinner.succeed("Succeeded!")

I know that this is possible to use a context manager, but this does require deeper nesting and is not that neat imo. Please let me know if you consider this a valid use case.

Description

This PR adds functionality to declare the optional keyword argument as a parameter to obtain a reference to the spinner object. This does not break anything and is completely optional.

Checklist

  • Your branch is up-to-date with the base branch
  • You've included at least one test if this is a new feature
  • All tests are passing

Further considerations

It is also possible to inject the spinner instance as a positional argument, but this could not be made optional.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 424

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at ?%

Totals Coverage Status
Change from base Build 422: 0.0%
Covered Lines:
Relevant Lines: 0

💛 - Coveralls

This pull request was closed.
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