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

Add ability to customize template keys #301

Merged
merged 2 commits into from Jul 13, 2021
Merged

Add ability to customize template keys #301

merged 2 commits into from Jul 13, 2021

Conversation

redzic
Copy link
Contributor

@redzic redzic commented Jul 6, 2021

This PR makes the pos and len fields of ProgressState to have pub visibility. I changed the per_sec method of ProgressState to return an f64, and {per_sec} in the template now rounds to an integer via the format! macro (rather than truncating by default), and is explicitly truncated in other places of the code. With this patch, you can now use .key(...) method from ProgressStyle to add a custom formatting function and template key. This PR also adds another example to show the usage of this feature.

I also changed the position method to just be 2 separate methods for returning the position and length, because the usage beforehand where position() was called before matching the template keys and then you would just reuse those for {pos} and {len} now seems like an anti-pattern with this hash map based approach.

Also I had to remove #[derive(Debug)] for ProgressStyle because I was getting errors of "Debug impl is not general enough" or something like that, and it didn't seem to be necessary anyway.

Copy link
Collaborator

@djc djc left a comment

Choose a reason for hiding this comment

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

Looking pretty good, I have a bunch of smallish suggestions.

examples/custom-template.rs Outdated Show resolved Hide resolved
src/progress_bar.rs Outdated Show resolved Hide resolved
src/state.rs Outdated Show resolved Hide resolved
src/state.rs Show resolved Hide resolved
src/state.rs Outdated Show resolved Hide resolved
src/style.rs Show resolved Hide resolved
src/style.rs Outdated Show resolved Hide resolved
src/style.rs Show resolved Hide resolved
@redzic redzic force-pushed the main branch 2 times, most recently from 14a24ed to 5e545f8 Compare July 9, 2021 18:01
@redzic
Copy link
Contributor Author

redzic commented Jul 9, 2021

@djc should be ready to review

src/progress_bar.rs Outdated Show resolved Hide resolved
src/style.rs Outdated Show resolved Hide resolved
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