Skip to content

Commit

Permalink
Migrate output towards the 1.0 strategy
Browse files Browse the repository at this point in the history
This commit is the first in what is hopefully a series to help move
`wasm-pack`'s CLI output and interactions to a "1.0 status". This was
[discussed at the recent Rust All Hands][discussion] where the salient
points we ended up extracting were:

* At all times if a user is waiting for `wasm-pack` to finish it should
  be clear what's being waited on.
  * As an example, Cargo's own output shows what crate is being built.
  * As another example, something that always takes only a handful of
    milliseconds to complete doesn't need an informational message.
* The final output products of a command should always be clear and
  printed. For example the output location of artifacts should always be
  printed.
* The `wasm-pack` CLI tool should use "progressive enhancement" to
  incrementally detect features of the output it can use (like colors,
  emoji, etc) but always work in the absence of these features. This'll
  help us support a wide range of use cases and terminals.

The goal of this commit is to not get us all the way there but start us
down the path to satisfying these goals. To that end the major change
here is to remove the dependency on `indicatif`. Using `indicatif`
requires that all output is piped through the `indicatif` crate itself,
which causes the third item here to not work for one of the main parts
of `wasm-pack build`, the `cargo` pieces. Cargo (and the Rust compiler)
are unable to use thir own tools for progressive enhancement when the
output is captured and sent through `indicatif`.

Lots more refactoring will be needed internally to fully polish off the
input/output to a "1.0 status", but this is hopefully a good start!

[discussion]: https://gist.github.com/fitzgen/23a62ebbd67574b9f6f72e5ac8eaeb67#file-road-to-wasm-pack-1-0-md
  • Loading branch information
alexcrichton committed Feb 21, 2019
1 parent a7e45d0 commit 2284f6b
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 110 deletions.
35 changes: 0 additions & 35 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ env_logger = { version = "0.5.13", default-features = false }
failure = "0.1.2"
human-panic = "1.0.1"
glob = "0.2"
indicatif = "0.9.0"
lazy_static = "1.1.0"
log = "0.4.6"
openssl = { version = '0.10.11', optional = true }
parking_lot = "0.6"
Expand All @@ -39,6 +37,7 @@ walkdir = "2"

[dev-dependencies]
assert_cmd = "0.10.2"
lazy_static = "1.1.0"
predicates = "1.0.0"
tempfile = "3"

Expand Down
3 changes: 1 addition & 2 deletions src/command/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use cache;
use command::utils::{create_pkg_dir, set_crate_path};
use emoji;
use failure::Error;
use indicatif::HumanDuration;
use license;
use lockfile::Lockfile;
use log::info;
Expand Down Expand Up @@ -204,7 +203,7 @@ impl Build {
step_counter.inc();
}

let duration = HumanDuration(started.elapsed());
let duration = crate::command::utils::elapsed(started.elapsed());
info!("Done in {}.", &duration);
info!(
"Your wasm pkg is ready to publish at {}.",
Expand Down
3 changes: 1 addition & 2 deletions src/command/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use command::utils::set_crate_path;
use console::style;
use emoji;
use failure::Error;
use indicatif::HumanDuration;
use lockfile::Lockfile;
use log::info;
use manifest;
Expand Down Expand Up @@ -172,7 +171,7 @@ impl Test {
process_step(&mut self, &step_counter)?;
step_counter.inc();
}
let duration = HumanDuration(started.elapsed());
let duration = crate::command::utils::elapsed(started.elapsed());
info!("Done in {}.", &duration);

Ok(())
Expand Down
12 changes: 12 additions & 0 deletions src/command/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use failure;
use progressbar::Step;
use std::fs;
use std::path::{Path, PathBuf};
use std::time::Duration;
use walkdir::WalkDir;
use PBAR;

Expand Down Expand Up @@ -39,3 +40,14 @@ pub fn find_pkg_directory(path: &Path) -> Option<PathBuf> {
fn is_pkg_directory(path: &Path) -> bool {
path.exists() && path.is_dir() && path.ends_with("pkg")
}

/// Render a `Duration` to a form suitable for display on a console
pub fn elapsed(duration: Duration) -> String {
let secs = duration.as_secs();

if secs >= 60 {
format!("{}m {:02}s", secs / 60, secs % 60)
} else {
format!("{}.{:02}s", secs, duration.subsec_nanos() / 10_000_000)
}
}
11 changes: 3 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@ extern crate strsim;
#[macro_use]
extern crate failure;
extern crate glob;
extern crate indicatif;
extern crate which;
#[macro_use]
extern crate lazy_static;
extern crate parking_lot;
extern crate serde;
extern crate which;
#[macro_use]
extern crate serde_derive;
extern crate serde_ignored;
Expand Down Expand Up @@ -43,10 +40,8 @@ pub mod test;

use progressbar::ProgressOutput;

lazy_static! {
/// The global progress bar and user-facing message output.
pub static ref PBAR: ProgressOutput = { ProgressOutput::new() };
}
/// The global progress bar and user-facing message output.
pub static PBAR: ProgressOutput = ProgressOutput;

/// 📦 ✨ pack and publish your wasm!
#[derive(Debug, StructOpt)]
Expand Down
64 changes: 3 additions & 61 deletions src/progressbar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,61 +2,26 @@

use console::style;
use emoji;
use indicatif::{ProgressBar, ProgressStyle};
use parking_lot::RwLock;
use std::fmt;

/// Synchronized progress bar and status message printing.
pub struct ProgressOutput {
spinner: RwLock<ProgressBar>,
messages: RwLock<String>,
}
pub struct ProgressOutput;

impl ProgressOutput {
/// Construct a new `ProgressOutput`.
pub fn new() -> Self {
Self {
spinner: RwLock::new(ProgressBar::new_spinner()),
messages: RwLock::new(String::from("")),
}
}

/// Inform the user that the given `step` is being executed, with details in
/// `message`.
pub fn step(&self, step: &Step, message: &str) {
let msg = format!("{} {}", style(step).bold().dim(), message);
self.message(&msg)
}

fn finish(&self) {
let spinner = self.spinner.read();
spinner.finish();

let mut message = self.messages.write();
print!("{}", *message);
message.clear();
}

/// Print the given message.
pub fn message(&self, message: &str) {
self.finish();

let mut spinner = self.spinner.write();
*spinner = Self::progressbar(message);

if !atty::is(atty::Stream::Stderr) {
// `indicatif` won't print any output if `stderr` is not a tty, so
// to ensure that our output is still emitted, we print it manually
// here.
eprintln!("{}", message)
}
eprintln!(" {}", message);
}

fn add_message(&self, msg: &str) {
let mut message = self.messages.write();
message.push_str(" ");
message.push_str(msg);
message.push('\n');
eprintln!("{}", msg);
}

/// Add an informational message.
Expand Down Expand Up @@ -91,23 +56,6 @@ impl ProgressOutput {
);
self.add_message(&err);
}

fn progressbar(msg: &str) -> ProgressBar {
let pb = ProgressBar::new_spinner();
pb.enable_steady_tick(200);
pb.set_style(
ProgressStyle::default_spinner()
.tick_chars("/|\\- ")
.template("{spinner:.dim.bold} {wide_msg}"),
);
pb.set_message(&msg);
pb
}

/// After having built up a series of messages, print all of them out.
pub fn done(&self) {
self.finish();
}
}

/// For processes that can be broken down into N fractional steps, with messages
Expand Down Expand Up @@ -136,9 +84,3 @@ impl fmt::Display for Step {
write!(f, "[{}/{}]", self.current, self.total)
}
}

impl Drop for ProgressOutput {
fn drop(&mut self) {
self.done();
}
}

0 comments on commit 2284f6b

Please sign in to comment.