Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/cli/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,17 @@ pub static ARCH: Lazy<String> = Lazy::new(|| {
.to_string()
});

pub static VERSION: Lazy<String> = Lazy::new(|| {
pub static VERSION_PLAIN: Lazy<String> = Lazy::new(|| {
let mut v = V.to_string();
if cfg!(debug_assertions) {
v.push_str("-DEBUG");
};
v
});

pub static VERSION: Lazy<String> = Lazy::new(|| {
let build_time = BUILD_TIME.format("%Y-%m-%d");
let v = &*VERSION_PLAIN;
format!("{v} {os}-{arch} ({build_time})", os = *OS, arch = *ARCH)
});

Expand Down
17 changes: 17 additions & 0 deletions src/toolset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ impl Toolset {
return Ok(vec![]);
}

// Initialize a header for the entire install session once (before batching)
let mpr = MultiProgressReport::get();
mpr.init_header("install", versions.len());

// Run pre-install hook
hooks::run_one_hook(config, self, Hooks::Preinstall, None).await;

Expand All @@ -230,6 +234,8 @@ impl Toolset {
successful_installations,
failed_installations,
}) => {
// Count both successes and failures toward header progress
mpr.header_inc(successful_installations.len() + failed_installations.len());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Progress Bar Shows Incorrect Progress

The progress bar can display incorrect or inflated progress. When install_some_versions returns an error, install_all_versions increments the global progress counter for tools that were already counted individually, leading to double-counting.

Additional Locations (1)
Fix in Cursor Fix in Web

installed.extend(successful_installations);
return Err(Error::InstallFailed {
successful_installations: installed,
Expand Down Expand Up @@ -279,6 +285,8 @@ impl Toolset {
// Run post-install hook (ignoring errors)
let _ = hooks::run_one_hook(config, self, Hooks::Postinstall, None).await;

// Finish the global header
mpr.header_finish();
Ok(installed)
}

Expand Down Expand Up @@ -315,6 +323,10 @@ impl Toolset {
}
};

// Initialize global header progress (overall tools)
let mpr = MultiProgressReport::get();
mpr.init_header("install", versions_clone.len());

// Track plugin installation errors to avoid early returns
let mut plugin_errors = Vec::new();

Expand Down Expand Up @@ -427,6 +439,8 @@ impl Toolset {
.await;

results.push((tr, result));
// Bump header for each completed tool
MultiProgressReport::get().header_inc(1);
}
results
});
Expand Down Expand Up @@ -467,6 +481,9 @@ impl Toolset {
}
}

// Finish header bar
MultiProgressReport::get().header_finish();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: Progress Bar Inaccuracies During Batched Installations

The header progress bar provides inaccurate progress: it re-initializes with batch sizes when install_all_versions is called in batches, and it fails to reach 100% when tools are filtered out due to plugin installation or semaphore acquisition errors. Additionally, an early return during queue building leaves the header unfinished.

Fix in Cursor Fix in Web


// Return appropriate result
if failed_installations.is_empty() {
Ok(successful_installations)
Expand Down
67 changes: 52 additions & 15 deletions src/ui/multi_progress_report.rs
Original file line number Diff line number Diff line change
@@ -1,35 +1,33 @@
use std::sync::{Arc, Mutex, Weak};
use std::sync::{Arc, Mutex};

use indicatif::MultiProgress;

use crate::config::Settings;
use crate::ui::progress_report::{ProgressReport, QuietReport, SingleReport, VerboseReport};
use crate::ui::progress_report::{
HeaderReport, ProgressReport, QuietReport, SingleReport, VerboseReport,
};
use crate::ui::style;
use crate::{cli::version::VERSION_PLAIN, config::Settings};

#[derive(Debug)]
pub struct MultiProgressReport {
mp: Option<MultiProgress>,
quiet: bool,
header: Mutex<Option<HeaderReport>>,
}

static INSTANCE: Mutex<Option<Weak<MultiProgressReport>>> = Mutex::new(None);
static INSTANCE: Mutex<Option<Arc<MultiProgressReport>>> = Mutex::new(None);

impl MultiProgressReport {
pub fn try_get() -> Option<Arc<Self>> {
match &*INSTANCE.lock().unwrap() {
Some(w) => w.upgrade(),
None => None,
}
INSTANCE.lock().unwrap().as_ref().cloned()
}
pub fn get() -> Arc<Self> {
let mut mutex = INSTANCE.lock().unwrap();
if let Some(w) = &*mutex {
if let Some(mpr) = w.upgrade() {
return mpr;
}
let mut guard = INSTANCE.lock().unwrap();
if let Some(existing) = guard.as_ref() {
return existing.clone();
}

let mpr = Arc::new(Self::new());
*mutex = Some(Arc::downgrade(&mpr));
*guard = Some(mpr.clone());

Copilot AI Aug 14, 2025

Copy link

Choose a reason for hiding this comment

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

Storing an Arc directly in the static instance creates a memory leak - the MultiProgressReport will never be dropped. Consider keeping the original Weak reference pattern to allow proper cleanup when no other references exist.

Copilot uses AI. Check for mistakes.
mpr
}
fn new() -> Self {
Expand All @@ -45,6 +43,7 @@ impl MultiProgressReport {
MultiProgressReport {
mp,
quiet: settings.quiet,
header: Mutex::new(None),
}
}
pub fn add(&self, prefix: &str) -> Box<dyn SingleReport> {
Expand All @@ -58,6 +57,44 @@ impl MultiProgressReport {
None => Box::new(VerboseReport::new(prefix.to_string())),
}
}
pub fn init_header(&self, label: &str, total_tools: usize) {
if self.mp.is_none() || self.quiet {
return;
}
let mut hdr = self.header.lock().unwrap();
match (&self.mp, hdr.as_ref()) {
(Some(mp), None) => {
let version = &*VERSION_PLAIN;
let prefix = format!(
"{} {} {}",
style::emagenta("mise").bold(),
style::edim(format!("{version} by @jdx –")),
style::eblue(label),
);
let mut header = HeaderReport::new(prefix);
header.pb = mp.add(header.pb);
header.set_length(total_tools as u64);
*hdr = Some(header);
}
(_, Some(_h)) => {
// header already initialized; do not change total
}
_ => {}
}
}
pub fn header_inc(&self, n: usize) {
if n == 0 {
return;
}
if let Some(h) = &*self.header.lock().unwrap() {
h.inc(n as u64);
}
}
pub fn header_finish(&self) {
if let Some(h) = &*self.header.lock().unwrap() {
h.finish_with_message("installed".to_string());
}
}
pub fn suspend_if_active<F: FnOnce() -> R, R>(f: F) -> R {
match Self::try_get() {
Some(mpr) => mpr.suspend(f),
Expand Down
74 changes: 65 additions & 9 deletions src/ui/progress_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ static SUCCESS_TEMPLATE: Lazy<ProgressStyle> = Lazy::new(|| {
ProgressStyle::with_template(tmpl.as_str()).unwrap()
});

static HEADER_TEMPLATE: Lazy<ProgressStyle> = Lazy::new(|| {
let width = match *env::TERM_WIDTH {
0..=79 => 10,
80..=99 => 15,
_ => 20,
};
let tmpl = format!(r#"{{prefix}} {{bar:{width}.cyan/blue}} {{pos}}/{{len:2}}"#);
ProgressStyle::with_template(&tmpl).unwrap()
});

#[derive(Debug)]
pub struct ProgressReport {
pub pb: ProgressBar,
Expand Down Expand Up @@ -118,16 +128,10 @@ impl SingleReport for ProgressReport {
self.pb.abandon();
}
fn finish(&self) {
self.pb.set_style(SUCCESS_TEMPLATE.clone());
self.pb
.set_prefix(success_prefix(self.pad - 2, &self.prefix));
self.pb.finish()
self.pb.finish_and_clear();
}
fn finish_with_message(&self, message: String) {
self.pb.set_style(SUCCESS_TEMPLATE.clone());
self.pb
.set_prefix(success_prefix(self.pad - 2, &self.prefix));
self.pb.finish_with_message(message);
fn finish_with_message(&self, _message: String) {
self.pb.finish_and_clear();
}
}

Expand Down Expand Up @@ -180,6 +184,58 @@ impl SingleReport for VerboseReport {
}
}

#[derive(Debug)]
pub struct HeaderReport {
pub pb: ProgressBar,
}

impl HeaderReport {
pub fn new(prefix: String) -> HeaderReport {
ui::ctrlc::show_cursor_after_ctrl_c();
let pb = ProgressBar::new(100)

Copilot AI Aug 14, 2025

Copy link

Choose a reason for hiding this comment

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

The magic number 100 should be defined as a named constant or explained with a comment. It's unclear why the initial length is set to 100 when it gets overridden by set_length().

Suggested change
let pb = ProgressBar::new(100)
// The initial length is arbitrary and will be overridden by set_length().
let pb = ProgressBar::new(DEFAULT_PROGRESS_LENGTH)

Copilot uses AI. Check for mistakes.
.with_style(HEADER_TEMPLATE.clone())
.with_prefix(prefix);
HeaderReport { pb }
}
}

impl SingleReport for HeaderReport {
fn println(&self, message: String) {
self.pb.suspend(|| {
eprintln!("{message}");
});
}
fn set_message(&self, message: String) {
self.pb.set_message(message.replace('\r', ""));
}
fn inc(&self, delta: u64) {
self.pb.inc(delta);
}
fn set_position(&self, pos: u64) {
self.pb.set_position(pos);
if Some(self.pb.position()) == self.pb.length() {

Copilot AI Aug 14, 2025

Copy link

Choose a reason for hiding this comment

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

This comparison will fail if pb.length() returns None. Use self.pb.length().map_or(false, |len| self.pb.position() == len) to handle the case where length is not set.

Suggested change
if Some(self.pb.position()) == self.pb.length() {
if self.pb.length().map_or(false, |len| self.pb.position() == len) {

Copilot uses AI. Check for mistakes.
self.pb.set_style(SPIN_TEMPLATE.clone());
self.pb.enable_steady_tick(Duration::from_millis(250));
}
}
fn set_length(&self, length: u64) {
self.pb.set_position(0);
self.pb.set_style(HEADER_TEMPLATE.clone());
self.pb.set_length(length);
}
fn abandon(&self) {
self.pb.abandon();
}
fn finish(&self) {
self.pb.set_style(SUCCESS_TEMPLATE.clone());
self.pb.finish()
}
fn finish_with_message(&self, message: String) {
self.pb.set_style(SUCCESS_TEMPLATE.clone());
self.pb.finish_with_message(message);
}
}

#[cfg(test)]
mod tests {
use crate::config::Config;
Expand Down
Loading