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

Added optional printer argument to spinner action #172

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

calebstewart
Copy link

@calebstewart calebstewart commented Mar 19, 2024

This PR adds an optional argument to the printer action which provides access to the tea.Printf and tea.Println functionality. I added the Printer interface with two methods: Printf and Println. I also added a accessiblePrinter type which just proxies the calls to fmt.Printf and fmt.Println.

When called normally (without accessibility), the tea.Program is passed in as the Printer object. When called with Accessible(true), then a dummy accessiblePrinter object is passed in.

The Spinner interface is left backwards compatible. The Action() method still takes a func(), but a new method is introduced called ActionWithPrinter() which takes a func(Printer) function instead.

@calebstewart calebstewart changed the title Add action printer Added optional printer argument to spinner action Mar 19, 2024
@maaslalani
Copy link
Contributor

Ohhh, this is a very nice idea and good implementation @calebstewart! Thank you so much.

Let me just think about what we should call this to make it more easily discoverable. I almost wonder if we could have people simply print to a buffer on the spinner to achieve this effect i.e. with fmt.Sprintln(spinner.Out, "This is a test") 🤔

@@ -21,14 +21,29 @@ import (
// ⣾ Loading...
type Spinner struct {
spinner spinner.Model
action func()
action func(Printer)
Copy link
Member

Choose a reason for hiding this comment

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

maybe passing a io.Writer is simpler 🤔

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.

3 participants