Skip to content

Commit

Permalink
Merge pull request #1054 from workingjubilee/fix-todo
Browse files Browse the repository at this point in the history
fix: todo, minor typos, extra clones
  • Loading branch information
drager authored Sep 5, 2021
2 parents 126dd74 + 69c5624 commit ca4af76
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 26 deletions.
18 changes: 9 additions & 9 deletions src/bindgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use failure::{self, ResultExt};
use install::{self, Tool};
use manifest::CrateData;
use semver;
use std::path::{Path, PathBuf};
use std::path::Path;
use std::process::Command;

/// Run the `wasm-bindgen` CLI to generate bindings for the current crate's
Expand Down Expand Up @@ -49,7 +49,7 @@ pub fn wasm_bindgen_build(
.arg(dts_arg);

let target_arg = build_target_arg(target, &bindgen_path)?;
if supports_dash_dash_target(bindgen_path.to_path_buf())? {
if supports_dash_dash_target(&bindgen_path)? {
cmd.arg("--target").arg(target_arg);
} else {
cmd.arg(target_arg);
Expand All @@ -75,7 +75,7 @@ pub fn wasm_bindgen_build(
}

/// Check if the `wasm-bindgen` dependency is locally satisfied for the web target
fn supports_web_target(cli_path: &PathBuf) -> Result<bool, failure::Error> {
fn supports_web_target(cli_path: &Path) -> Result<bool, failure::Error> {
let cli_version = semver::Version::parse(&install::get_cli_version(
&install::Tool::WasmBindgen,
cli_path,
Expand All @@ -85,30 +85,30 @@ fn supports_web_target(cli_path: &PathBuf) -> Result<bool, failure::Error> {
}

/// Check if the `wasm-bindgen` dependency is locally satisfied for the --target flag
fn supports_dash_dash_target(cli_path: PathBuf) -> Result<bool, failure::Error> {
fn supports_dash_dash_target(cli_path: &Path) -> Result<bool, failure::Error> {
let cli_version = semver::Version::parse(&install::get_cli_version(
&install::Tool::WasmBindgen,
&cli_path,
cli_path,
)?)?;
let expected_version = semver::Version::parse("0.2.40")?;
Ok(cli_version >= expected_version)
}

fn build_target_arg(target: Target, cli_path: &PathBuf) -> Result<String, failure::Error> {
if !supports_dash_dash_target(cli_path.to_path_buf())? {
fn build_target_arg(target: Target, cli_path: &Path) -> Result<String, failure::Error> {
if !supports_dash_dash_target(cli_path)? {
Ok(build_target_arg_legacy(target, cli_path)?)
} else {
Ok(target.to_string())
}
}

fn build_target_arg_legacy(target: Target, cli_path: &PathBuf) -> Result<String, failure::Error> {
fn build_target_arg_legacy(target: Target, cli_path: &Path) -> Result<String, failure::Error> {
log::info!("Your version of wasm-bindgen is out of date. You should consider updating your Cargo.toml to a version >= 0.2.40.");
let target_arg = match target {
Target::Nodejs => "--nodejs",
Target::NoModules => "--no-modules",
Target::Web => {
if supports_web_target(&cli_path)? {
if supports_web_target(cli_path)? {
"--web"
} else {
bail!("Your current version of wasm-bindgen does not support the 'web' target. Please update your project to wasm-bindgen version >= 0.2.39.")
Expand Down
4 changes: 2 additions & 2 deletions src/build/wasm_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use emoji;
use failure::{Error, ResultExt};
use log::info;
use std::fmt;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::process::Command;
use PBAR;

Expand Down Expand Up @@ -81,7 +81,7 @@ fn get_rustc_sysroot() -> Result<PathBuf, Error> {
}

/// Checks if the wasm32-unknown-unknown is present in rustc's sysroot.
fn is_wasm32_target_in_sysroot(sysroot: &PathBuf) -> bool {
fn is_wasm32_target_in_sysroot(sysroot: &Path) -> bool {
let wasm32_target = "wasm32-unknown-unknown";

let rustlib_path = sysroot.join("lib/rustlib");
Expand Down
6 changes: 3 additions & 3 deletions src/command/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl Build {
let path = build_opts.path.take().unwrap();
build_opts
.extra_options
.insert(0, path.to_string_lossy().into_owned().to_string());
.insert(0, path.to_string_lossy().into_owned());
}
}
let crate_path = get_crate_path(build_opts.path)?;
Expand Down Expand Up @@ -370,7 +370,7 @@ impl Build {
let bindgen = install::download_prebuilt_or_cargo_install(
Tool::WasmBindgen,
&self.cache,
&bindgen_version,
bindgen_version,
self.mode.install_permitted(),
)?;
self.bindgen = Some(bindgen);
Expand All @@ -382,7 +382,7 @@ impl Build {
info!("Building the wasm bindings...");
bindgen::wasm_bindgen_build(
&self.crate_data,
&self.bindgen.as_ref().unwrap(),
self.bindgen.as_ref().unwrap(),
&self.out_dir,
&self.out_name,
self.disable_dts,
Expand Down
2 changes: 1 addition & 1 deletion src/emoji.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//! Emoji contants used by `wasm-pack`.
//! Emoji constants used by `wasm-pack`.
//!
//! For the woefully unfamiliar:
//!
Expand Down
10 changes: 5 additions & 5 deletions src/install/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use log::debug;
use log::{info, warn};
use std::env;
use std::fs;
use std::path::PathBuf;
use std::path::Path;
use std::process::Command;
use target;
use which::which;
Expand Down Expand Up @@ -71,7 +71,7 @@ pub fn download_prebuilt_or_cargo_install(
let msg = format!("{}Installing {}...", emoji::DOWN_ARROW, tool);
PBAR.info(&msg);

let dl = download_prebuilt(&tool, &cache, version, install_permitted);
let dl = download_prebuilt(&tool, cache, version, install_permitted);
match dl {
Ok(dl) => return Ok(dl),
Err(e) => {
Expand All @@ -82,13 +82,13 @@ pub fn download_prebuilt_or_cargo_install(
}
}

cargo_install(tool, &cache, version, install_permitted)
cargo_install(tool, cache, version, install_permitted)
}

/// Check if the tool dependency is locally satisfied.
pub fn check_version(
tool: &Tool,
path: &PathBuf,
path: &Path,
expected_version: &str,
) -> Result<bool, failure::Error> {
let expected_version = if expected_version == "latest" {
Expand All @@ -107,7 +107,7 @@ pub fn check_version(
}

/// Fetches the version of a CLI tool
pub fn get_cli_version(tool: &Tool, path: &PathBuf) -> Result<String, failure::Error> {
pub fn get_cli_version(tool: &Tool, path: &Path) -> Result<String, failure::Error> {
let mut cmd = Command::new(path);
cmd.arg("--version");
let stdout = child::run_capture_stdout(cmd, tool)?;
Expand Down
2 changes: 1 addition & 1 deletion src/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
//! downloaded via curl/sh, and then the shell script downloads this executable
//! and runs it.
//!
//! This may get more complicated over time (self upates anyone?) but for now
//! This may get more complicated over time (self updates anyone?) but for now
//! it's pretty simple! We're largely just moving over our currently running
//! executable to a different path.
Expand Down
2 changes: 1 addition & 1 deletion src/manifest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ impl CrateData {
files,
main: js_file,
homepage: self.manifest.package.homepage.clone(),
keywords: keywords,
keywords,
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/test/webdriver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ fn get_and_notify(
name: &str,
url: &str,
) -> Result<Option<PathBuf>, failure::Error> {
if let Some(dl) = cache.download(false, name, &[name], &url)? {
if let Some(dl) = cache.download(false, name, &[name], url)? {
return Ok(Some(dl.binary(name)?));
}
if installation_allowed {
PBAR.info(&format!("Getting {}...", name));
}
match cache.download(installation_allowed, name, &[name], &url)? {
match cache.download(installation_allowed, name, &[name], url)? {
Some(dl) => Ok(Some(dl.binary(name)?)),
None => Ok(None),
}
Expand All @@ -39,8 +39,7 @@ struct Collector(Vec<u8>);

impl Collector {
pub fn take_content(&mut self) -> Vec<u8> {
// TODO: replace with `std::mem::take` once stable
std::mem::replace(&mut self.0, Vec::default())
std::mem::take(&mut self.0)
}
}

Expand Down

0 comments on commit ca4af76

Please # to comment.