pb icon indicating copy to clipboard operation
pb copied to clipboard

[WIP] Logging and more

Open stbuehler opened this issue 6 years ago • 4 comments

As requested in #44.

I split it up into many single commit, some of them not breaking the API, not all of them needed for logging.

The ProgressReceiver part (and the final logging part) will break the API - maybe it would be better to have a separate "next" branch for those changes (I have some more ideas :D).

Windows not supported yet - maybe someone working on windows can help out.

stbuehler avatar Jul 27 '17 15:07 stbuehler

Now that I think of it, I didn't actually use the ProgressReceiver trait to implement logging - I only implemented logging for MultiBar, and used a separate "frontend" for the channel...

But it is possible to add a log method to the ProgressReceiver trait (I even added it in my first try, just didn't use it anywhere) and make it available through ProgressBar, although implementing Write on ProgressBar with it would conflict with the current semantics.

Should ProgressBar also support logging? How should the interface look like? Just a log method, or a Write implementation?

The current ("byte counting for progress") Write implementation for ProgressBar should imho be removed anyway, and be implemented on a wrapper type instead if needed.

stbuehler avatar Jul 28 '17 05:07 stbuehler

Thanks for your contribution @stbuehler ! before I review it, can you please add an example to the example directory, so I'll be able to run it?

a8m avatar Jul 29 '17 10:07 a8m

I'll try to come up with something simple.

Just to clarify: only the last commit actually adds the logging; the others before can be reviewed without the logging example :)

I also thought about using a similar patch without breaking the API. And then realized how broken the current implementation is:

  • finish* doesn't print a newline. Following data just happens to be on a new line because it fills the line - unless of course the width calculation broke, the terminal got resized, or someone tries to copy&paste from the terminal.
  • finish_print with MultiBar leaks resources (it doesn't count as "done" in MultiBar)

This could probably be fixed by printing newlines in PB, and removing them in MultiBar - but is it worth it to fix this?

stbuehler avatar Aug 01 '17 18:08 stbuehler

Hi, I just added a commit providing an example and added another fix to the list, and removed the Write implementation for ProgressBar.

I also worked on a branch which doesn't break the API, but is based on the same fixes (#57) as this one: https://github.com/stbuehler/rust-pbr/tree/logging-keep-api

Basically the API changes could be done in a separate "next version" branch, or be dropped completely.

stbuehler avatar Aug 28 '17 17:08 stbuehler