Skip to content
This repository has been archived by the owner on May 20, 2020. It is now read-only.

Implement --quiet flag #166

Merged
merged 4 commits into from
Aug 27, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
41 changes: 24 additions & 17 deletions src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use std::io::prelude::*;
use std::path::Path;
use std::process::{Command, Stdio};

use indicatif::ProgressBar;
use serde_json;

use error::*;
use ui::Ui;
Copy link
Owner

Choose a reason for hiding this comment

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

👏


/// The kinds of targets that we can document.
#[derive(Debug, PartialEq, Eq)]
Expand Down Expand Up @@ -77,11 +77,11 @@ pub fn retrieve_metadata(manifest_path: &Path) -> Result<serde_json::Value> {
///
/// - `manifest_path`: The path to the crate's Cargo.toml
/// - `target`: The target that we should generate the analysis data for
pub fn generate_analysis(
manifest_path: &Path,
target: &Target,
progress: &ProgressBar,
) -> Result<()> {
/// - `report_progress`: A closure that should be called to report a progress message
pub fn generate_analysis<F>(manifest_path: &Path, target: &Target, report_progress: F) -> Result<()>
where
F: Fn(&str) -> (),
{
let mut command = Command::new("cargo");

let target_dir = manifest_path
Expand Down Expand Up @@ -132,7 +132,7 @@ pub fn generate_analysis(
if line.starts_with("Updating") || line.starts_with("Compiling") ||
line.starts_with("Finished")
{
progress.set_message(line);
report_progress(line);
}
}
}
Expand All @@ -151,7 +151,7 @@ pub fn generate_analysis(
/// ## Arguments
///
/// - metadata: The JSON metadata of the crate.
pub fn target_from_metadata(metadata: &serde_json::Value) -> Result<Target> {
pub fn target_from_metadata(ui: &Ui, metadata: &serde_json::Value) -> Result<Target> {
// We can expect at least one package and target, otherwise the metadata generation would have
// failed.
let targets = metadata["packages"][0]["targets"].as_array().expect(
Expand Down Expand Up @@ -196,30 +196,37 @@ pub fn target_from_metadata(metadata: &serde_json::Value) -> Result<Target> {
Ok(targets.remove(0))
} else {
// FIXME(#105): Handle more than one target.
print!("warning: Found more than one target to document. ");
let (mut libs, mut bins): (Vec<_>, Vec<_>) =
targets.into_iter().partition(|target| match target.kind {
TargetKind::Library => true,
TargetKind::Binary => false,
});

if !libs.is_empty() {
println!("Documenting the library.");
ui.warn(
"Found more than one target to document. Documenting the library.",
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be good to say which library

Copy link
Contributor

@mgattozzi mgattozzi Aug 25, 2017

Choose a reason for hiding this comment

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

Actually disregard you can only have one library, but having the actual name would be useful

);
Ok(libs.remove(0))
} else {
let target = bins.remove(0);
println!("Documenting the first binary: {}", target.name);
ui.warn(&format!(
"Found more than one target to document. Documenting the first binary: {}",
target.name
));
Ok(target)
}
}
}

#[cfg(test)]
mod tests {
use ui::Ui;
use super::{Target, TargetKind};

#[test]
fn target_from_metadata() {
let ui = Ui::default();

let metadata = json!({
"packages": [
{
Expand All @@ -233,7 +240,7 @@ mod tests {
},
],
});
let target = super::target_from_metadata(&metadata).unwrap();
let target = super::target_from_metadata(&ui, &metadata).unwrap();
assert_eq!(target, Target { kind: TargetKind::Library, name: "underscored_name".into() });
assert_eq!(&target.crate_name(), "underscored_name");

Expand All @@ -250,7 +257,7 @@ mod tests {
},
],
});
let target = super::target_from_metadata(&metadata).unwrap();
let target = super::target_from_metadata(&ui, &metadata).unwrap();
assert_eq!(target, Target { kind: TargetKind::Library, name: "dashed-name".into() });
assert_eq!(&target.crate_name(), "dashed_name");

Expand All @@ -267,7 +274,7 @@ mod tests {
},
],
});
let target = super::target_from_metadata(&metadata).unwrap();
let target = super::target_from_metadata(&ui, &metadata).unwrap();
assert_eq!(target, Target { kind: TargetKind::Binary, name: "underscored_name".into() });
assert_eq!(&target.crate_name(), "underscored_name");

Expand All @@ -284,7 +291,7 @@ mod tests {
},
],
});
assert_eq!(super::target_from_metadata(&metadata).unwrap().kind, TargetKind::Library);
assert_eq!(super::target_from_metadata(&ui, &metadata).unwrap().kind, TargetKind::Library);

let metadata = json!({
"packages": [
Expand All @@ -299,7 +306,7 @@ mod tests {
},
],
});
assert_eq!(super::target_from_metadata(&metadata).unwrap().kind, TargetKind::Binary);
assert_eq!(super::target_from_metadata(&ui, &metadata).unwrap().kind, TargetKind::Binary);

let metadata = json!({
"packages": [
Expand All @@ -318,6 +325,6 @@ mod tests {
},
],
});
assert_eq!(super::target_from_metadata(&metadata).unwrap().kind, TargetKind::Library);
assert_eq!(super::target_from_metadata(&ui, &metadata).unwrap().kind, TargetKind::Library);
}
}
61 changes: 27 additions & 34 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,27 @@ pub mod cargo;
pub mod error;
pub mod json;

mod ui;

pub use json::create_json;

use std::fs::{self, File};
use std::io::prelude::*;
use std::path::{Path, PathBuf};

use indicatif::{ProgressBar, ProgressStyle};

use assets::Asset;
use cargo::Target;
use error::*;
use ui::Ui;

pub use error::{Error, ErrorKind};

/// A structure that contains various fields that hold data in order to generate doc output.
#[derive(Debug)]
pub struct Config {
/// Interactions with the user interface.
ui: Ui,

/// Path to the `Cargo.toml` file for the crate being analyzed
manifest_path: PathBuf,

Expand All @@ -56,14 +60,15 @@ impl Config {
/// ## Arguments
///
/// - `manifest_path`: The path to the `Cargo.toml` of the crate being documented
pub fn new(manifest_path: PathBuf, assets: Vec<Asset>) -> Result<Config> {
pub fn new(quiet: bool, manifest_path: PathBuf, assets: Vec<Asset>) -> Result<Config> {
Copy link
Owner

Choose a reason for hiding this comment

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

maybe this should be an enum of levels instead; quiet, verbose, and normal?

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. What if in the future we want -vvv? Having an enum would make that easier and is more type safe. It'll express what it does better.

let host = analysis::AnalysisHost::new(analysis::Target::Debug);

if !manifest_path.is_file() || !manifest_path.ends_with("Cargo.toml") {
bail!("The --manifest-path must be a path to a Cargo.toml file");
}

Ok(Config {
ui: Ui::new(quiet),
manifest_path,
host,
assets,
Expand Down Expand Up @@ -97,54 +102,39 @@ impl Config {
/// - `artifacts`: A slice containing what assets should be output at the end
pub fn build(config: &Config, artifacts: &[&str]) -> Result<()> {
let metadata = cargo::retrieve_metadata(&config.manifest_path)?;
let target = cargo::target_from_metadata(&metadata)?;
let target = cargo::target_from_metadata(&config.ui, &metadata)?;
generate_and_load_analysis(config, &target)?;

let output_path = config.output_path();
fs::create_dir_all(&output_path)?;

if artifacts.contains(&"json") {
let spinner = create_spinner();
spinner.set_prefix("Generating JSON");
spinner.set_message("In Progress");
let task = config.ui.start_task("Generating JSON");
task.report("In Progress");

let json = create_json(&config.host, &target.crate_name())?;

let json_path = output_path.join("data.json");
let mut file = File::create(json_path)?;
file.write_all(json.as_bytes())?;
spinner.finish_with_message("Done");
}

// now that we've written out the data, we can write out the rest of it
if artifacts.contains(&"assets") {
let spinner = create_spinner();
spinner.set_prefix("Copying Assets");
spinner.set_message("In Progress");
let task = config.ui.start_task("Copying Assets");
task.report("In Progress");

let assets_path = output_path.join("assets");
fs::create_dir_all(&assets_path)?;

for asset in &config.assets {
assets::create_asset_file(asset.name, &output_path, asset.contents)?;
}

spinner.finish_with_message("Done");
}

Ok(())
}

/// Create a new spinner to display progress to the user.
fn create_spinner() -> ProgressBar {
let spinner = ProgressBar::new_spinner();
spinner.enable_steady_tick(50);
spinner.set_style(ProgressStyle::default_spinner().template(
"{spinner} {prefix}: {wide_msg}",
));
spinner
}

/// Generate save analysis data of a crate to be used later by the RLS library later and load it
/// into the analysis host.
///
Expand All @@ -156,23 +146,26 @@ fn create_spinner() -> ProgressBar {
fn generate_and_load_analysis(config: &Config, target: &Target) -> Result<()> {
let manifest_path = &config.manifest_path;

let spinner = create_spinner();
spinner.set_prefix("Generating save analysis data");
spinner.set_message("In Progress");
let task = config.ui.start_task("Generating save analysis data");
task.report("In progress");

let analysis_result =
cargo::generate_analysis(manifest_path, target, |progress| { task.report(progress); });

if let Err(e) = cargo::generate_analysis(manifest_path, target, &spinner) {
spinner.finish_with_message("Error");
return Err(e);
if analysis_result.is_err() {
task.error();
return analysis_result;
}

spinner.finish_with_message("Done");
drop(task);

let task = config.ui.start_task("Loading save analysis data");
task.report("In Progress");

let spinner = create_spinner();
spinner.set_prefix("Loading save analysis data");
spinner.set_message("In Progress");
let root_path = config.root_path();
config.host.reload(root_path, root_path)?;
spinner.finish_with_message("Done");

drop(task);

Ok(())
}
5 changes: 4 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ fn run() -> rustdoc::error::Result<()> {
.default_value("./Cargo.toml")
.help("The path to the Cargo manifest of the project you are documenting."),
)
.arg(Arg::with_name("quiet").short("q").long("quiet").help(
"No output printed to stdout.",
))
.subcommand(
SubCommand::with_name("build")
.about("generates documentation")
Expand All @@ -54,7 +57,7 @@ fn run() -> rustdoc::error::Result<()> {
// unwrap is okay because we take a default value
let manifest_path = PathBuf::from(&matches.value_of("manifest-path").unwrap());
let assets = include!(concat!(env!("OUT_DIR"), "/asset.in"));
let config = Config::new(manifest_path, assets)?;
let config = Config::new(matches.is_present("quiet"), manifest_path, assets)?;

match matches.subcommand() {
("build", Some(matches)) => {
Expand Down
80 changes: 80 additions & 0 deletions src/ui.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use std::cell::Cell;
use std::fmt::{self, Debug};

use indicatif::{ProgressBar, ProgressStyle};

#[derive(Debug, Default)]
pub struct Ui {
quiet: bool,
}

impl Ui {
pub fn new(quiet: bool) -> Ui {
Ui { quiet }
}

pub fn start_task(&self, name: &str) -> Task {
let spinner = if self.quiet {
ProgressBar::hidden()
} else {
ProgressBar::new_spinner()
};

spinner.enable_steady_tick(50);
spinner.set_style(ProgressStyle::default_spinner().template(
"{spinner} {prefix}: {wide_msg}",
));

spinner.set_prefix(name);

Task {
ui: self,
spinner,
is_error: Cell::new(false),
}
}

pub fn warn(&self, message: &str) {
if !self.quiet {
eprintln!("warning: {}", message);
}
}
}

pub struct Task<'a> {
ui: &'a Ui,
spinner: ProgressBar,
is_error: Cell<bool>,
}

impl<'a> Debug for Task<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
f.debug_struct("Task")
.field("ui", &self.ui)
.field("spinner", &String::from("Spinner {}"))
.field("is_error", &self.is_error)
.finish()
}
}

impl<'a> Task<'a> {
pub fn report(&self, message: &str) {
self.spinner.set_message(message);
}

pub fn error(&self) {
self.is_error.set(true);
}
}

impl<'a> Drop for Task<'a> {
fn drop(&mut self) {
let message = if self.is_error.take() {
"Error"
} else {
"Done"
};

self.spinner.finish_with_message(message);
}
}
2 changes: 1 addition & 1 deletion tests/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod validation_tests {

#[test]
fn json_fmt_test() {
let config = Config::new(PathBuf::from("example/Cargo.toml"), vec![])
let config = Config::new(true, PathBuf::from("example/Cargo.toml"), vec![])
.unwrap_or_else(|err| {
panic!("Couldn't create the config: {}", err);
});
Expand Down