feat: add progress bars for time-consuming operations #1249

Eshaan-byte · 2025-12-04T04:38:58Z

This PR implements visual progress feedback for all time-consuming Dora CLI operations using the indicatif crate.

Changes:

Created shared progress bar component library (progress.rs)
Added progress to build, up, start, and run commands
Unified cyan/blue styling with success/failure indicators
Smart TTY detection (auto-hides in CI environments)
Multi-progress support for parallel operations

All operations >2s now have visual feedback with consistent behavior.

Testing:

cargo check: passed
cargo fmt: passed
cargo clippy: passed

Closes dora-rs#1229 This PR implements visual progress feedback for all time-consuming Dora CLI operations using the indicatif crate. Changes: - Created shared progress bar component library (progress.rs) - Added progress to build, up, start, and run commands - Unified cyan/blue styling with success/failure indicators - Smart TTY detection (auto-hides in CI environments) - Multi-progress support for parallel operations All operations >2s now have visual feedback with consistent behavior. Testing: - cargo check: passed - cargo fmt: passed - cargo clippy: passed

phil-opp

Thanks for the PR!

binaries/cli/Cargo.toml

binaries/cli/src/progress.rs

binaries/cli/src/command/build/local.rs

This commit addresses the remaining review feedback from PR dora-rs#1249: 1. Replace `atty` with `std::io::IsTerminal`: - Removed deprecated `atty` crate dependency - Use `std::io::stderr().is_terminal()` for TTY detection 2. Rewrite progress bar documentation: - Simplified module docs to focus on behavior and usage - Added progress bar example (not just spinners) - Clarified behavior in non-TTY environments - Removed marketing terms 3. Add progress bar example: - Added example showing progress bar with known steps 4. Remove unused code: - Removed `should_show_progress()` function - Removed `ProgressBarBuilder` struct and tests 5. Refactor to reduce duplication: - Created `new_inner()` helper method - Both `new()` and `new_bytes()` now use shared implementation 6. Fix MultiProgress in try_clone(): - Store MultiProgress reference in LocalBuildLogger - Use multi.add_spinner() in try_clone() to properly add to MultiProgress 7. Keep log messages visible: - Always print log messages to stdout for scrollback - Also update progress bar message for live feedback - Users can now scroll up to see warnings/errors

Eshaan-byte · 2025-12-11T01:26:08Z

Hi @phil-opp, Thank you for the detailed review! I've addressed all the feedback points:
-Replaced atty with std::io::IsTerminal

Rewrote the documentation to be clearer and removed marketing terms
-Added progress bar example (not just spinner)
Removed unused should_show_progress() function
Refactored new()/new_bytes() with shared new_inner() helper
Removed unused ProgressBarBuilder
Fixed try_clone() to properly add progress bars to MultiProgress
Log messages are now always printed to stdout for scrollback, while also updating the progress bar
Please let me know if there's anything else that needs adjustment!

phil-opp

Thanks for the updates!

binaries/cli/src/command/build/local.rs

Addresses review feedback about try_clone() implementation: 1. Fixed bug where cloning a progress bar without MultiProgress would result in None instead of preserving the progress bar 2. Now preserves the original progress bar's message instead of hardcoding a new message format 3. Handles three cases properly: - With MultiProgress: adds new spinner to multi with same message - Without MultiProgress: creates standalone spinner with same message - No progress bar: returns None The try_clone() method is used at libraries/core/src/build/mod.rs:125 when spawning tokio tasks to log stdout from build commands.

phil-opp · 2025-12-16T08:34:35Z

binaries/cli/src/command/build/local.rs

+        let progress_bar = match (&self.progress_bar, &self.multi) {
+            (Some(pb), Some(multi)) => {
+                // Clone the progress bar by creating a new one with the same message
+                // and adding it to the MultiProgress
+                let message = pb.inner().message();
+                Some(multi.add_spinner(message))
+            }
+            (Some(pb), None) => {
+                // No MultiProgress, create a standalone progress bar with the same message
+                let message = pb.inner().message();
+                Some(ProgressBar::new_spinner(message))
+            }
+            _ => None,
+        };
+
        Ok(LocalBuildLogger {
            node_id: self.node_id.clone(),
+            progress_bar,
+            multi: self.multi.clone(),


I'm still not sure about this implementation. It's still always a spinner, but I guess that's ok since we don't create progress bars in LocalBuildLogger. The bigger issue is that creating a new ProgressBar displays another bar, doesn't it?

Why not store the bar as an Arc<ProgressBar>? Then we could cheaply clone it without duplicating the bar.

This addresses the PR feedback by wrapping ProgressBar in Arc instead of creating new progress bars in try_clone(). This approach: - Avoids displaying duplicate progress bars when cloning - Provides cheap clones through Arc's reference counting - Simplifies the try_clone() implementation significantly The previous implementation created new progress bars on each clone, which would result in multiple visual indicators for the same task.

Eshaan-byte · 2025-12-17T10:42:52Z

PTAL @phil-opp, I have implemented the changes that you mentioned. Thank you for the guidance.

phil-opp

Thanks for the updates!

The code looks good to me now, but the output format still needs improvement imo.

For me, the dora build output looks like this currently:

So the percentage is printed a lot and makes the output quite messy. I would prefer if we could "pin" the progress bar and the spinners to the bottom so that they aren't part of the messages that scroll up. So basically a region at the bottom where we display progress and another region above where we print the log/stdout messages.

For example, here is how cargo does it:

The Building progress marker at the bottom stays there and does not interfere with the "Compiling" lines above.

Eshaan-byte · 2025-12-17T12:51:11Z

Hi @phil-opp, Thanks for the feedback!
I want to make sure I understand the desired behavior correctly: Currently, I'm printing log messages with println! (line 134 in local.rs) AND updating the progress bar message.
Should I:
Remove the println! statements entirely and only show progress via the bars/spinners (letting indicatif keep them pinned at the bottom), Or
Keep both but configure indicatif differently so progress bars stay at the bottom while logs scroll above?
With option 1, users won't see the detailed build logs in real-time (only in scrollback if we redirect them somehow). With option 2, I need to understand how to configure the output format. Which approach matches your vision? Thanks!

phil-opp · 2025-12-17T13:15:06Z

The main requirement is that the output stays readable. Before this PR, the output looked like this:

With this PR, it looks like this:

I think you agree that the output before was more readable.

phil-opp · 2025-12-17T13:55:27Z

It looks like pinning the progress bar is indicatif's default behavior, but you have to use the ProgressBar::println or MultiProgress::println functions instead of normal println. These methods don't do anything on non-terminal targets though, so we should fall back to normal println in that case (to still see the output in logs).

This addresses phil-opp's feedback about output readability by using indicatif's MultiProgress::println instead of regular println!. Changes: - Added MultiProgress::println method with fallback for non-TTY - Updated log_message to use MultiProgress::println - Progress bars now stay pinned at bottom while logs scroll above - Non-terminal outputs (CI/logs) still work via println! fallback This matches cargo's behavior where progress indicators stay at the bottom and don't interfere with scrolling log messages.

Eshaan-byte · 2025-12-18T01:39:32Z

Hi @phil-opp, I have implemented the changes.

Deepak-negi11 · 2026-01-11T15:50:20Z

@phil-opp

I see this at the beginning if this is not a problem i think the pr is good to go . You can check it again if i have miss somthing

Setting the message before adding it to `MultiProgress` draws the progress bar directly, which messes up the rendering of the multi progress bar. This leads to duplicated progress bars in the output.

phil-opp · 2026-01-12T09:22:28Z

@Deepak-negi11 Thanks for checking! I pushed eb1fdf0 to hopefully fix this issue. Could you try again?

phil-opp

Thanks for the updates!

phil-opp

I noticed that we still see a lot of spinners in the logs for dora run:

This needs to be fixed too before we can merge.

phil-opp reviewed Dec 10, 2025

View reviewed changes

phil-opp reviewed Dec 11, 2025

View reviewed changes

binaries/cli/src/command/build/local.rs Outdated Show resolved Hide resolved

phil-opp reviewed Dec 16, 2025

View reviewed changes

phil-opp reviewed Dec 17, 2025

View reviewed changes

phil-opp mentioned this pull request Jan 10, 2026

Provide progress bars for time-consuming operations #1229

Open

Fix: Set progress bar messages only after adding them to MultiProgress

eb1fdf0

Setting the message before adding it to `MultiProgress` draws the progress bar directly, which messes up the rendering of the multi progress bar. This leads to duplicated progress bars in the output.

phil-opp approved these changes Jan 12, 2026

View reviewed changes

phil-opp enabled auto-merge January 12, 2026 09:27

phil-opp requested changes Jan 12, 2026

View reviewed changes

phil-opp added the waiting-for-author The pull request requires adjustments by the PR author. label Jan 28, 2026

feat: add progress bars for time-consuming operations #1249

Are you sure you want to change the base?

feat: add progress bars for time-consuming operations #1249

Uh oh!

Conversation

Eshaan-byte commented Dec 4, 2025 • edited by phil-opp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

phil-opp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Eshaan-byte commented Dec 11, 2025

Uh oh!

phil-opp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phil-opp Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Eshaan-byte commented Dec 17, 2025

Uh oh!

phil-opp left a comment

Choose a reason for hiding this comment

Uh oh!

Eshaan-byte commented Dec 17, 2025

Uh oh!

phil-opp commented Dec 17, 2025

Uh oh!

phil-opp commented Dec 17, 2025

Uh oh!

Eshaan-byte commented Dec 18, 2025

Uh oh!

Deepak-negi11 commented Jan 11, 2026

Uh oh!

phil-opp commented Jan 12, 2026

Uh oh!

phil-opp left a comment

Choose a reason for hiding this comment

Uh oh!

phil-opp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Eshaan-byte commented Dec 4, 2025 •

edited by phil-opp

Loading