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

Implement Clone for ProgressDrawTarget #532

Closed
wants to merge 1 commit into from

Conversation

dfaust
Copy link
Contributor

@dfaust dfaust commented Apr 16, 2023

Uses dyn-clone to make ProgressDrawTarget and TermLike cloneable.

@djc
Copy link
Collaborator

djc commented Apr 16, 2023

What's your use case for this?

@dfaust
Copy link
Contributor Author

dfaust commented Apr 16, 2023

This allows creating multiple progress bars within one handler, if a ProgressDrawTarget is provided.
In my case, I'm watching a directory for changes and want to draw a new progress bar whenever changes are detected.

@djc
Copy link
Collaborator

djc commented Apr 16, 2023

Why not use a MultiProgress for that?

@dfaust
Copy link
Contributor Author

dfaust commented Apr 16, 2023

In the case I mentioned, this would be possible.
But it requires to pass the progress bar down to the functions that use it, instead of the draw target.
I don't think that's a clean solution. I'm passing the draw target as part of a context object, and I don't want to create a progress bar just in case I might need it at some point.
Another use case (that I'm not using right now) would be, to print messages between progress bars.

Are you concerned about the additional dependency? In that case I could make it optional.

@djc
Copy link
Collaborator

djc commented Apr 18, 2023

So why do you need to pass the ProgressDrawTarget anyway?

@dfaust
Copy link
Contributor Author

dfaust commented Apr 18, 2023

So why do you need to pass the ProgressDrawTarget anyway?

I use it for testing.

@djc
Copy link
Collaborator

djc commented Apr 19, 2023

Okay, that doesn't do anything to convince me that this is the right solution for the problem you're trying to address. Despite me asking several questions, you've yet to state a clear picture of what it is you're trying to achieve that you think necessitates passing around a ProgressDrawTarget which doesn't seem like a common way to use our API. If you would still like to have changes made in this project, please open an issue that outlines, at a conceptual level, what you are trying to do and why other solutions aren't acceptable for this use case.

@djc djc closed this Apr 19, 2023
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