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

feat: custom writer for progressbar and spinner printers #337

Conversation

Brookke
Copy link
Contributor

@Brookke Brookke commented Apr 2, 2022

Description

Adds a custom writer to the progress bar and spinner components.

Scope

What is affected by this pull request?
ProgressBar and Spinner

  • Bug Fix
  • New Feature
  • Documentation
  • Other

Related Issue

Fixes #261

To-Do Checklist

  • I tested my changes
  • I have commented every method that I created/changed
  • I updated the examples to fit with my changes
  • I have added tests for my newly created methods

@Brookke Brookke changed the title Issue 261 custom writer for progress spinners Feat: custom writer for progressbar and spinner printers Apr 2, 2022
@Brookke Brookke changed the title Feat: custom writer for progressbar and spinner printers feat: custom writer for progressbar and spinner printers Apr 2, 2022
@codecov
Copy link

codecov bot commented Apr 2, 2022

Codecov Report

Merging #337 (4ee46ab) into master (11fc7cd) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #337   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         1505      1509    +4     
=========================================
+ Hits          1505      1509    +4     
Impacted Files Coverage Δ
print.go 100.00% <100.00%> (ø)
progressbar_printer.go 100.00% <100.00%> (ø)
spinner_printer.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 11fc7cd...4ee46ab. Read the comment docs.

|[Add(count int)](https://pkg.go.dev/github.com/pterm/pterm#TemplatePrinter.Add)|Add `count` to current value.|
|[GetElapsedTime()](https://pkg.go.dev/github.com/pterm/pterm#TemplatePrinter.GetElapsedTime)|GetElapsedTime returns the elapsed time, since the ProgressbarPrinter was started.|
|[Increment()](https://pkg.go.dev/github.com/pterm/pterm#TemplatePrinter.Increment)|Increment current value by one.|
|[UpdateTitle(title string)](https://pkg.go.dev/github.com/pterm/pterm#TemplatePrinter.UpdateTitle)|Update the progressbar's title.|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These links seemed wrong so I updated them too :)

@lowdewijk
Copy link

Exactly what I need! Was actually considering doing this myself. 👍

@MarvinJWendt
Copy link
Member

Hi @Brookke, thanks for the PR! Could you maybe add some tests to ensure, the output is actually written to the given writer? You can do that with testza: https://github.com/MarvinJWendt/testza#capturestderr

You could use the writer in the callback function to write to and assert the output contains a word you put into the progressbar / spinner title. Thanks!

Copy link
Member

@MarvinJWendt MarvinJWendt left a comment

Choose a reason for hiding this comment

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

Some more unit tests, that capture the output and ensure that a specific writer is used, are missing. If you need help with testza, just ping me here or on Discord :)

@Brookke
Copy link
Contributor Author

Brookke commented Apr 11, 2022

@MarvinJWendt I have added some tests for the spinner printer, and I will do the same for the progress bar soon. However I had to add time.sleeps in order to wait for the go routines to run, which isn't great. If you have any other suggestions do let me know

@Brookke
Copy link
Contributor Author

Brookke commented Apr 11, 2022

Additionally it looks like the tests fail on ubuntu, I will investigate why

@MarvinJWendt
Copy link
Member

@MarvinJWendt I have added some tests for the spinner printer, and I will do the same for the progress bar soon. However I had to add time.sleeps in order to wait for the go routines to run, which isn't great. If you have any other suggestions do let me know

The CI-System takes ages to complete already, so I don't think the second delay is a problem, as I also can't think of an easy solution.

@Brookke
Copy link
Contributor Author

Brookke commented Apr 12, 2022

@MarvinJWendt great thanks, I'll do the same for the progress back early next week, currently on holiday :)

Copy link
Member

@MarvinJWendt MarvinJWendt left a comment

Choose a reason for hiding this comment

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

I hope you enjoyed your holidays! The PR looks good to me!

@MarvinJWendt MarvinJWendt merged commit e95b01c into pterm:master Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ability to set custom io.writter for each of the different output levels
3 participants