-
Notifications
You must be signed in to change notification settings - Fork 56
Add CLI progress bars #239
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
base: master
Are you sure you want to change the base?
Conversation
16b915a to
8b6b44e
Compare
e8fa8c5 to
35a73db
Compare
0dfe99a to
b02ae96
Compare
First compiling version sess: Fix compiler warnings progress: Get rid of the `Sub` prefix for submodules progress: Clean up a bit
git: Don't report progress when fetching tags progress: Don't set length multiple times progress: Refactor progress: Don't allow clones of Progresshandlers Clones of progress bars don't really work well after they were finished git: Don't clone progress handler Format sources
progress: Print duration of git operation progress: Stylistic changes progress: Add `Submodule` operations progress: Improve time formatting progress: Mention submodules in completed message progress: Improve display for submodules progress: Stylistic changes for submodules
error: Add formatting macros error: Align `stageln` to other macros error: Rename `Note` to `Info`
progress: Don't check for ASCII characters progress: Change the order of functions, enums and impl blocks progress: Clean up and document
fd145e3 to
995ee75
Compare
micprog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice feature, providing some insight into what bender is doing at the moment! Some comments below.
62c0c8f to
0949dda
Compare
|
I should have addressed all the comments now. Additionally, I added the following:
|
progress: Color in progress operations in cyan progress: Reword in progress messages
Whenever we are not in a TTY/terminal (e.g. a CI), we don't actually want to render the progress bar, but just print out the completion messages in the end for the log.
Is now in the standard library
In newer versions of git, tags are automatically fetched if the belong to the history of the reference that is fetched (i.e. a branch). Fetching `--tags` is therefore in most cases a no-op, since tags have been fetched in the first git operation. Only in the case where a specific branch is fetched e.g. `git fetch origin my_branch`, fetching additional tags from other branches could make sense. This is not the case here, since we fetch the entire remote
38ae3ac to
dcf1660
Compare
micprog
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! 👍
|
CI failures look to be slight formatting adjustments in the |
CLI Progress bars
Adds progress bars to bender actions that require longer network access. A taste is shown below*
*some style changes have been done since this recording.
Implementation
The progress bars are rendered with the crate
indicatif, that allows rendering of multiple progress bars at once. The state of the progress bars is extracted from the stderr output of the spawned git commands i.e. a separate task is spawned alongside git tasks to parse git output lines and update the progress bar state. I initially thought about adopting thegit2crate, which would have built in progress callbacks. However, this would have been a larger change and also poses some challenges since those git tasks are not asynchronous and also credential handling would be more complicated, which is why I opted for stderr parsing in the end.Related Changes
Apart from the implementation of the progress bars, some additional changes were necessary:
errormacrosPreviously, all messages were simply printed to stdout/stderr with
println!/eprintln!without any specific order. Rendering the progress bars together with warning/error messaged caused unwanted artifacts. The reason is that the progress bar requires exclusive access over stdout/stderr, since it needs to clear lines again. If warning/error messages are sent in between, it might clear the warning/error message instead of the actual progress bar which is left behind as an artifact. For that matter, theMultiProgressinstance can be suspended, which essentially means clearing all progress bars, writing the desired custom message to stdout/stderr and then redraw all the progress bars. This means the previouswarnln!,errorln!etc., macros are now passed through theMultiProgress.suspend()function.gitspawnsspawnfunctions now accept aOption<ProgressHandler>argument. IfNoneis passed, the behaviour is as before, whereasSome(pb)will add a new progress bar.git submodule updatecommands are only performed if a dependency contains a.gitmodules. Otherwise, too many no-op progress messages are spawned.TODOs
checkout, but others use the same underlying functions so there should not be an issue in theory.