Let the spinners spin again

This will fix the broken spinners that indicate steps which are
currently in progress. To achieve this the behavior of the progressbar
helper had to be adjusted.

The multibar does not work while a child process is run with
`std::process::Command`. Using multiple spinners without a multibar is
problematic as well because emitting warnings, infos and errors while a
single spinner is active will duplicate the spinners message. The
spinner will also absorb the last warning/error/info. Instead of
publishing warnings, errors and infos immediately they will now be
cached and only output when the current spinner finishes.

To make sure we can output all warnings to the user ProgressBars are no
longer exposed to the caller. Instead the active spinner will be
finished implicitly when a new spinner is allocated with the `message()`
function. Instead of relying on the `done()` function to be called the
progressbar now implements the `Drop` trait which will finish the last
spinner automatically.

Unfortunately, this introduces members that have to be mutable. To not
have to deal with mutable ProgressOutput all over the place the members
were put inside an RwLock. This allows us to use ProgressOutput from
inside multiple threads to emit warnings and errors.

src/bindgen.rs

@@ -6,17 +6,16 @@ use PBAR;
 
 pub fn cargo_install_wasm_bindgen(step: &Step) -> Result<(), Error> {
     let msg = format!("{}Installing WASM-bindgen...", emoji::DOWN_ARROW);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let output = Command::new("cargo")
         .arg("install")
         .arg("wasm-bindgen-cli")
         .arg("--force")
         .output()?;
-    pb.finish();
     if !output.status.success() {
         let s = String::from_utf8_lossy(&output.stderr);
         if s.contains("already exists") {
-            PBAR.info("wasm-bindgen already installed");
+            PBAR.info("wasm-bindgen already installed")?;
             return Ok(());
         }
         Error::cli("Installing wasm-bindgen failed", s)
@@ -34,7 +33,7 @@ pub fn wasm_bindgen_build(
     step: &Step,
 ) -> Result<(), Error> {
     let msg = format!("{}Running WASM-bindgen...", emoji::RUNNER);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let binary_name = name.replace("-", "_");
     let release_or_debug = if debug { "debug" } else { "release" };
     let wasm_path = format!(
@@ -60,7 +59,6 @@ pub fn wasm_bindgen_build(
         .arg(dts_arg)
         .arg(target_arg)
         .output()?;
-    pb.finish();
     if !output.status.success() {
         let s = String::from_utf8_lossy(&output.stderr);
         Error::cli("wasm-bindgen failed to execute properly", s)

src/build.rs

@@ -6,7 +6,7 @@ use PBAR;
 
 pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> {
     let msg = format!("{}Adding WASM target...", emoji::TARGET);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     ensure_nightly()?;
     let output = Command::new("rustup")
         .arg("target")
@@ -15,7 +15,6 @@ pub fn rustup_add_wasm_target(step: &Step) -> Result<(), Error> {
         .arg("--toolchain")
         .arg("nightly")
         .output()?;
-    pb.finish();
     if !output.status.success() {
         let s = String::from_utf8_lossy(&output.stderr);
         Error::cli("Adding the wasm32-unknown-unknown target failed", s)
@@ -42,7 +41,7 @@ fn ensure_nightly() -> Result<(), Error> {
 
 pub fn cargo_build_wasm(path: &str, debug: bool, step: &Step) -> Result<(), Error> {
     let msg = format!("{}Compiling to WASM...", emoji::CYCLONE);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let output = {
         let mut cmd = Command::new("cargo");
         cmd.current_dir(path).arg("+nightly").arg("build");
@@ -52,7 +51,7 @@ pub fn cargo_build_wasm(path: &str, debug: bool, step: &Step) -> Result<(), Erro
         cmd.arg("--target").arg("wasm32-unknown-unknown");
         cmd.output()?
     };
-    pb.finish();
+
     if !output.status.success() {
         let s = String::from_utf8_lossy(&output.stderr);
         Error::cli("Compilation of your program failed", s)

src/command.rs

@@ -146,15 +146,10 @@ pub fn run_wasm_pack(command: Command, log: &Logger) -> result::Result<(), Error
         Ok(_) => {}
         Err(ref e) => {
             error!(&log, "{}", e);
-            PBAR.error(e.error_type());
+            PBAR.error(e.error_type())?;
         }
     }
 
-    // Make sure we always clear the progress bar before we abort the program otherwise
-    // stderr and stdout output get eaten up and nothing will work. If this part fails
-    // to work and clear the progress bars then you're really having a bad day with your tools.
-    PBAR.done()?;
-
     // Return the actual status of the program to the main function
     status
 }
@@ -164,10 +159,9 @@ pub fn run_wasm_pack(command: Command, log: &Logger) -> result::Result<(), Error
 // the correct type here.
 pub fn create_pkg_dir(path: &str, step: &Step) -> result::Result<(), Error> {
     let msg = format!("{}Creating a pkg directory...", emoji::FOLDER);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let pkg_dir_path = format!("{}/pkg", path);
     fs::create_dir_all(pkg_dir_path)?;
-    pb.finish();
     Ok(())
 }
 
@@ -257,13 +251,13 @@ impl Init {
             "Your WASM pkg is ready to publish at {}/pkg.", &self.crate_path
         );
 
-        PBAR.message(&format!("{} Done in {}", emoji::SPARKLE, &duration));
+        PBAR.message(&format!("{} Done in {}", emoji::SPARKLE, &duration))?;
 
         PBAR.message(&format!(
             "{} Your WASM pkg is ready to publish at {}/pkg.",
             emoji::PACKAGE,
             &self.crate_path
-        ));
+        ))?;
         Ok(())
     }
 
@@ -420,7 +414,7 @@ fn pack(path: Option<String>, log: &Logger) -> result::Result<(), Error> {
     #[cfg(target_os = "windows")]
     info!(&log, "Your package is located at {}\\pkg", &crate_path);
 
-    PBAR.message("🎒  packed up your package!");
+    PBAR.message("🎒  packed up your package!")?;
     Ok(())
 }
 
@@ -432,7 +426,7 @@ fn publish(path: Option<String>, log: &Logger) -> result::Result<(), Error> {
     npm::npm_publish(&crate_path)?;
     info!(&log, "Published your package!");
 
-    PBAR.message("💥  published your package!");
+    PBAR.message("💥  published your package!")?;
     Ok(())
 }
 
@@ -458,7 +452,7 @@ fn login(
     npm::npm_login(&registry, &scope, always_auth, &auth_type)?;
     info!(&log, "Logged you in!");
 
-    PBAR.message(&format!("👋  logged you in!"));
+    PBAR.message(&format!("👋  logged you in!"))?;
     Ok(())
 }
 

src/error.rs

@@ -2,6 +2,7 @@
 use serde_json;
 use std::borrow::Cow;
 use std::io;
+use std::sync::PoisonError;
 use toml;
 
 #[derive(Debug, Fail)]
@@ -13,6 +14,8 @@ pub enum Error {
     SerdeJson(#[cause] serde_json::Error),
     #[fail(display = "{}", _0)]
     SerdeToml(#[cause] toml::de::Error),
+    #[fail(display = "Acquiring lock failed")]
+    PoisonedLockError,
     #[fail(display = "{}. stderr:\n\n{}", message, stderr)]
     Cli { message: String, stderr: String },
     #[fail(display = "{}", message)]
@@ -38,6 +41,7 @@ impl Error {
             Error::Io(_) => "There was an I/O error. Details:\n\n",
             Error::SerdeJson(_) => "There was an JSON error. Details:\n\n",
             Error::SerdeToml(_) => "There was an TOML error. Details:\n\n",
+            Error::PoisonedLockError => "There was an RwLock error. Details: \n\n",
             Error::Cli {
                 message: _,
                 stderr: _,
@@ -66,3 +70,9 @@ impl From<toml::de::Error> for Error {
         Error::SerdeToml(e)
     }
 }
+
+impl<T> From<PoisonError<T>> for Error {
+    fn from(_: PoisonError<T>) -> Self {
+        Error::PoisonedLockError
+    }
+}

src/manifest.rs

@@ -123,25 +123,24 @@ pub fn write_package_json(
         )
     };
 
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let pkg_file_path = format!("{}/pkg/package.json", path);
     let mut pkg_file = File::create(pkg_file_path)?;
     let crate_data = read_cargo_toml(path)?;
     let npm_data = crate_data.into_npm(scope, disable_dts);
 
     if npm_data.description.is_none() {
-        PBAR.warn(&warn_fmt("description"));
+        PBAR.warn(&warn_fmt("description"))?;
     }
     if npm_data.repository.is_none() {
-        PBAR.warn(&warn_fmt("repository"));
+        PBAR.warn(&warn_fmt("repository"))?;
     }
     if npm_data.license.is_none() {
-        PBAR.warn(&warn_fmt("license"));
+        PBAR.warn(&warn_fmt("license"))?;
     }
 
     let npm_json = serde_json::to_string_pretty(&npm_data)?;
     pkg_file.write_all(npm_json.as_bytes())?;
-    pb.finish();
     Ok(())
 }
 

src/progressbar.rs

@@ -1,60 +1,90 @@
 use console::style;
 use emoji;
 use error::Error;
-use indicatif::{MultiProgress, ProgressBar, ProgressStyle};
+use indicatif::{ProgressBar, ProgressStyle};
 use std::fmt;
+use std::sync::RwLock;
 
 pub struct ProgressOutput {
-    bar: MultiProgress,
+    spinner: RwLock<ProgressBar>,
+    messages: RwLock<String>,
 }
 
 impl ProgressOutput {
     pub fn new() -> Self {
         Self {
-            bar: MultiProgress::new(),
+            spinner: RwLock::new(ProgressBar::new_spinner()),
+            messages: RwLock::new(String::from("")),
         }
     }
 
-    pub fn step(&self, step: &Step, message: &str) -> ProgressBar {
+    pub fn step(&self, step: &Step, message: &str) -> Result<(), Error> {
         let msg = format!("{} {}", style(step).bold().dim(), message);
-        self.bar.add(Self::progressbar(&msg))
+        self.message(&msg)
     }
 
-    pub fn message(&self, message: &str) -> ProgressBar {
-        self.bar.add(Self::progressbar(message))
+    fn finish(&self) -> Result<(), Error> {
+        let spinner = self.spinner.read()?;
+        spinner.finish();
+
+        let mut message = self.messages.write()?;
+        print!("{}", message);
+        message.clear();
+
+        Ok(())
+    }
+
+    pub fn message(&self, message: &str) -> Result<(), Error> {
+        self.finish()?;
+
+        let mut spinner = self.spinner.write()?;
+        *spinner = Self::progressbar(message);
+        Ok(())
+    }
+
+    fn add_message(&self, msg: &str) -> Result<(), Error> {
+        let mut message = self.messages.write()?;
+        message.push_str("  ");
+        message.push_str(msg);
+        message.push('\n');
+
+        Ok(())
     }
 
-    pub fn info(&self, message: &str) {
+    pub fn info(&self, message: &str) -> Result<(), Error> {
         let info = format!(
             "{} {}: {}",
             emoji::INFO,
             style("[INFO]").bold().dim(),
             message
         );
-        let bar = self.bar.add(Self::progressbar(&info));
-        bar.finish();
+        self.add_message(&info)?;
+
+        Ok(())
     }
 
-    pub fn warn(&self, message: &str) {
+    pub fn warn(&self, message: &str) -> Result<(), Error> {
         let warn = format!(
             "{} {}: {}",
             emoji::WARN,
             style("[WARN]").bold().dim(),
             message
         );
-        let bar = self.bar.add(Self::progressbar(&warn));
-        bar.finish();
+        self.add_message(&warn)?;
+
+        Ok(())
     }
 
-    pub fn error(&self, message: String) {
+    pub fn error(&self, message: String) -> Result<(), Error> {
         let err = format!(
             "{} {}: {}",
             emoji::ERROR,
             style("[ERR]").bold().dim(),
             message
         );
-        let bar = self.bar.add(Self::progressbar(&err));
-        bar.finish();
+        self.add_message(&err)?;
+
+        Ok(())
     }
 
     fn progressbar(msg: &str) -> ProgressBar {
@@ -70,7 +100,8 @@ impl ProgressOutput {
     }
 
     pub fn done(&self) -> Result<(), Error> {
-        self.bar.join_and_clear().map_err(|e| Error::from(e))
+        self.finish()?;
+        Ok(())
     }
 }
 
@@ -93,3 +124,9 @@ impl fmt::Display for Step {
         write!(f, "[{}/{}]", self.current, self.total)
     }
 }
+
+impl Drop for ProgressOutput {
+    fn drop(&mut self) {
+        self.done().unwrap();
+    }
+}

src/readme.rs

@@ -7,12 +7,11 @@ use PBAR;
 
 pub fn copy_from_crate(path: &str, step: &Step) -> Result<(), Error> {
     let msg = format!("{}Copying over your README...", emoji::DANCERS);
-    let pb = PBAR.step(step, &msg);
+    PBAR.step(step, &msg)?;
     let crate_readme_path = format!("{}/README.md", path);
     let new_readme_path = format!("{}/pkg/README.md", path);
     if let Err(_) = fs::copy(&crate_readme_path, &new_readme_path) {
-        PBAR.warn("origin crate has no README");
+        PBAR.warn("origin crate has no README")?;
     };
-    pb.finish();
     Ok(())
 }