Skip to content

Commit 9151190

Browse files
committed
Improve environment variable handling and user feedback
Fix environment variable export format to properly handle empty variables by using parameter expansion syntax. This prevents issues when PATH or similar variables are unset initially. Add progress indicators for git cloning operations to provide better user feedback during long-running fetch operations. The spinner shows activity while maintaining clean output in non-interactive environments. Enhance archive handling to support direct file copying when output filename is specified, avoiding unnecessary extraction for single files. Add caching awareness by logging when cached downloads are reused instead of re-downloading. Standardize progress message formatting across all operations for consistency, removing trailing periods and adjusting verb tenses to match established patterns.
1 parent 7287b2d commit 9151190

File tree

2 files changed

+78
-11
lines changed

2 files changed

+78
-11
lines changed

src/cli.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ fn handle_env_command(sprout_path: &str, command: EnvCommand) -> Result<()> {
628628

629629
for (var, path) in &package.exports {
630630
let full_path = dist_path.join(path.trim_start_matches('/'));
631-
println!("export {}=\"{}:${{{}}}\"", var, full_path.display(), var);
631+
println!("export {}=\"${{{}:+${{{}}}:}}{}\"", var, var, var, full_path.display());
632632
}
633633
}
634634
}

src/core/deps.rs

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ pub fn build_package(
392392

393393
if let Some(pb) = pb {
394394
pb.finish_and_clear();
395-
println!(" ✓ Built {}.", module_id);
395+
println!(" ✓ Built {}", module_id);
396396
}
397397

398398
// Update lockfile
@@ -437,6 +437,8 @@ pub fn get_dist_path(sprout_path: &str, package: &ModuleBlock) -> PathBuf {
437437

438438
fn fetch_git(sprout_path: &str, package: &ModuleBlock, git: &crate::ast::GitSpec) -> Result<()> {
439439
use std::process::Command;
440+
use indicatif::{ProgressBar, ProgressStyle};
441+
use std::time::Duration;
440442

441443
let source_path = get_source_path(sprout_path, package);
442444

@@ -455,7 +457,18 @@ fn fetch_git(sprout_path: &str, package: &ModuleBlock, git: &crate::ast::GitSpec
455457
let log_filename = format!("{}-fetch-{}.log", package.id(), timestamp);
456458
let log_path = logs_dir.join(&log_filename);
457459

458-
info!("Cloning git repository: {} -> {}", git.url, source_path.display());
460+
let pb = if atty::is(atty::Stream::Stderr) {
461+
let pb = ProgressBar::new_spinner();
462+
pb.set_style(ProgressStyle::default_spinner()
463+
.template(" {spinner} {msg}")?);
464+
pb.set_message(format!("Cloning {}", package.id()));
465+
pb.enable_steady_tick(Duration::from_millis(100));
466+
Some(pb)
467+
} else {
468+
info!("Cloning git repository: {} -> {}", git.url, source_path.display());
469+
None
470+
};
471+
459472
info!("Fetch log: {}", log_path.display());
460473

461474
// Log git clone command
@@ -490,6 +503,13 @@ fn fetch_git(sprout_path: &str, package: &ModuleBlock, git: &crate::ast::GitSpec
490503
.stderr(fs::OpenOptions::new().append(true).open(&log_path)?)
491504
.status()?;
492505

506+
if let Some(pb) = pb {
507+
pb.finish_and_clear();
508+
if status.success() {
509+
println!(" ✓ Cloned {}", package.id());
510+
}
511+
}
512+
493513
if !status.success() {
494514
return Err(anyhow!(
495515
"git clone failed with exit code: {:?}\nLog saved to: {}",
@@ -516,6 +536,8 @@ fn fetch_archive(sprout_path: &str, package: &ModuleBlock, archive: &crate::ast:
516536

517537
if !cache_path.exists() {
518538
download_file(&archive.url, &cache_path, original_filename)?;
539+
} else {
540+
info!("Using cached {}", original_filename);
519541
}
520542

521543
// Compute SHA256 if not present in manifest
@@ -560,8 +582,18 @@ fn fetch_archive(sprout_path: &str, package: &ModuleBlock, archive: &crate::ast:
560582
.map(|s| s.as_str())
561583
.unwrap_or(original_filename);
562584

563-
info!("Extracting {} -> {}", original_filename, source_path.display());
564-
extract_archive_with_output(&cache_path, &source_path, original_filename, output_filename)?;
585+
// If output is specified, skip extraction (just copy the file)
586+
let skip_extract = package.fetch.as_ref()
587+
.and_then(|f| f.output.as_ref())
588+
.is_some();
589+
590+
if skip_extract {
591+
info!("Copying {} -> {}", original_filename, source_path.display());
592+
copy_file_with_progress(&cache_path, &source_path, original_filename, output_filename)?;
593+
} else {
594+
info!("Extracting {} -> {}", original_filename, source_path.display());
595+
extract_archive_with_output(&cache_path, &source_path, original_filename, output_filename)?;
596+
}
565597
Ok(())
566598
}
567599

@@ -577,7 +609,7 @@ fn download_file(url: &str, dest: &Path, filename: &str) -> Result<()> {
577609
pb.set_style(ProgressStyle::default_bar()
578610
.template(" {msg} [{bar:40}] {bytes}/{total_bytes} ({eta})")?
579611
.progress_chars("=>-"));
580-
pb.set_message(format!("Download {}", filename));
612+
pb.set_message(format!("Downloading {}", filename));
581613
Some(pb)
582614
} else {
583615
info!("Downloading {}", filename);
@@ -599,7 +631,7 @@ fn download_file(url: &str, dest: &Path, filename: &str) -> Result<()> {
599631
}
600632

601633
if let Some(pb) = pb {
602-
pb.finish_with_message(format!("✓ Download {}", filename));
634+
pb.finish_with_message(format!("✓ Downloaded {}", filename));
603635
} else {
604636
info!("Downloaded {}", filename);
605637
}
@@ -627,19 +659,54 @@ fn compute_file_sha256(path: &Path) -> Result<String> {
627659
Ok(format!("{:x}", hasher.finalize()))
628660
}
629661

662+
fn copy_file_with_progress(cache_path: &Path, dest_dir: &Path, filename: &str, output_name: &str) -> Result<()> {
663+
use indicatif::{ProgressBar, ProgressStyle};
664+
use std::time::Duration;
665+
666+
let pb = if atty::is(atty::Stream::Stderr) {
667+
let pb = ProgressBar::new_spinner();
668+
pb.set_style(ProgressStyle::default_spinner()
669+
.template(" {spinner} {msg}")?);
670+
pb.set_message(format!("Copying {}", filename));
671+
pb.enable_steady_tick(Duration::from_millis(100));
672+
Some(pb)
673+
} else {
674+
info!("Copying {}", filename);
675+
None
676+
};
677+
678+
let output_path = dest_dir.join(output_name);
679+
std::fs::copy(cache_path, &output_path)?;
680+
681+
if let Some(pb) = pb {
682+
pb.finish_and_clear();
683+
println!(" ✓ Copied {}", filename);
684+
} else {
685+
info!("Copied {}", filename);
686+
}
687+
Ok(())
688+
}
689+
630690
fn extract_archive_with_output(cache_path: &Path, dest: &Path, filename: &str, output_name: &str) -> Result<()> {
631691
use indicatif::{ProgressBar, ProgressStyle};
632692
use std::time::Duration;
633693

694+
let is_archive = filename.ends_with(".tar.gz") || filename.ends_with(".tgz")
695+
|| filename.ends_with(".tar.xz") || filename.ends_with(".zip")
696+
|| filename.ends_with(".gz") || filename.ends_with(".xz");
697+
698+
let action = if is_archive { "Extracting" } else { "Copying" };
699+
let action_past = if is_archive { "Extracted" } else { "Copied" };
700+
634701
let pb = if atty::is(atty::Stream::Stderr) {
635702
let pb = ProgressBar::new_spinner();
636703
pb.set_style(ProgressStyle::default_spinner()
637704
.template(" {spinner} {msg}")?);
638-
pb.set_message(format!("Extract {}", filename));
705+
pb.set_message(format!("{} {}", action, filename));
639706
pb.enable_steady_tick(Duration::from_millis(100));
640707
Some(pb)
641708
} else {
642-
info!("Extracting {}", filename);
709+
info!("{} {}", action, filename);
643710
None
644711
};
645712

@@ -677,9 +744,9 @@ fn extract_archive_with_output(cache_path: &Path, dest: &Path, filename: &str, o
677744

678745
if let Some(pb) = pb {
679746
pb.finish_and_clear();
680-
println!(" ✓ Extracted {}.", filename);
747+
println!(" ✓ {} {}", action_past, filename);
681748
} else {
682-
info!("Extracted {}", filename);
749+
info!("{} {}", action_past, filename);
683750
}
684751
Ok(())
685752
}

0 commit comments

Comments
 (0)