Use open crate to open files and URLs

Opening URLs on Windows is very complex, so delegate to the
`open` crate.

type: changed

Cargo.lock

@@ -330,6 +330,7 @@ dependencies = [
  "libc",
  "log",
  "md5",
+ "open",
  "pretty_assertions",
  "pretty_env_logger",
  "regex",
@@ -447,6 +448,15 @@ version = "0.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "17b02fc0ff9a9e4b35b3342880f48e896ebf69f2967921fe8646bf5b7125956a"
 
+[[package]]
+name = "open"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7c283bf0114efea9e42f1a60edea9859e8c47528eae09d01df4b29c1e489cc48"
+dependencies = [
+ "winapi 0.3.8",
+]
+
 [[package]]
 name = "output_vt100"
 version = "0.1.2"

Cargo.toml

@@ -22,6 +22,7 @@ lazy_static       = "1.4.0"
 libc              = "0.2.0"
 log               = "0.4.8"
 md5               = "0.7.0"
+open              = "1.4.0"
 pretty_assertions = "0.6.0"
 pretty_env_logger = "0.4.0"
 regex             = "1.0.0"

src/common.rs

@@ -15,7 +15,7 @@ pub(crate) use std::{
   num::{ParseFloatError, ParseIntError, TryFromIntError},
   ops::{AddAssign, Div, DivAssign, Mul, MulAssign, Sub, SubAssign},
   path::{self, Path, PathBuf},
-  process::{self, Command, ExitStatus},
+  process::{self, ExitStatus},
   str::{self, FromStr},
   sync::Once,
   time::{SystemTime, SystemTimeError},
@@ -79,8 +79,8 @@ mod test {
     cell::RefCell,
     io::Cursor,
     ops::{Deref, DerefMut},
+    process::Command,
     rc::Rc,
-    time::{Duration, Instant},
   };
 
   // test dependencies

src/error.rs

@@ -7,23 +7,6 @@ use structopt::clap;
 pub(crate) enum Error {
   #[snafu(display("Failed to parse announce URL: {}", source))]
   AnnounceUrlParse { source: url::ParseError },
-  #[snafu(display("Failed to deserialize torrent metainfo from {}: {}", input, source))]
-  MetainfoDeserialize {
-    source: bendy::serde::Error,
-    input: InputTarget,
-  },
-  #[snafu(display("Failed to serialize torrent metainfo: {}", source))]
-  MetainfoSerialize { source: bendy::serde::Error },
-  #[snafu(display("Failed to decode torrent metainfo from {}: {}", input, error))]
-  MetainfoDecode {
-    input: InputTarget,
-    error: bendy::decoding::Error,
-  },
-  #[snafu(display("Metainfo from {} failed to validate: {}", input, source))]
-  MetainfoValidate {
-    input: InputTarget,
-    source: MetainfoError,
-  },
   #[snafu(display("Failed to parse byte count `{}`: {}", text, source))]
   ByteParse {
     text: String,
@@ -45,8 +28,6 @@ pub(crate) enum Error {
   Filesystem { source: io::Error, path: PathBuf },
   #[snafu(display("Invalid glob: {}", source))]
   GlobParse { source: globset::Error },
-  #[snafu(display("Unknown lint: {}", text))]
-  LintUnknown { text: String },
   #[snafu(display("Failed to serialize torrent info dictionary: {}", source))]
   InfoSerialize { source: bendy::serde::Error },
   #[snafu(display(
@@ -55,8 +36,29 @@ pub(crate) enum Error {
     message,
   ))]
   Internal { message: String },
-  #[snafu(display("Failed to find opener utility, please install one of {}", tried.join(",")))]
-  OpenerMissing { tried: &'static [&'static str] },
+  #[snafu(display("Unknown lint: {}", text))]
+  LintUnknown { text: String },
+  #[snafu(display("Failed to deserialize torrent metainfo from {}: {}", input, source))]
+  MetainfoDeserialize {
+    source: bendy::serde::Error,
+    input: InputTarget,
+  },
+  #[snafu(display("Failed to serialize torrent metainfo: {}", source))]
+  MetainfoSerialize { source: bendy::serde::Error },
+  #[snafu(display("Failed to decode torrent metainfo from {}: {}", input, error))]
+  MetainfoDecode {
+    input: InputTarget,
+    error: bendy::decoding::Error,
+  },
+  #[snafu(display("Metainfo from {} failed to validate: {}", input, source))]
+  MetainfoValidate {
+    input: InputTarget,
+    source: MetainfoError,
+  },
+  #[snafu(display("Failed to invoke opener: {}", source))]
+  OpenerInvoke { source: io::Error },
+  #[snafu(display("Opener failed: {}", exit_status))]
+  OpenerExitStatus { exit_status: ExitStatus },
   #[snafu(display("Output path already exists: `{}`", path.display()))]
   OutputExists { path: PathBuf },
   #[snafu(display(

src/platform.rs

@@ -4,15 +4,6 @@ pub(crate) struct Platform;
 
 #[cfg(target_os = "windows")]
 impl PlatformInterface for Platform {
-  fn opener() -> Result<Vec<OsString>, Error> {
-    let exe = if cfg!(test) { "open.bat" } else { "cmd" };
-    Ok(vec![
-      OsString::from(exe),
-      OsString::from("/C"),
-      OsString::from("start"),
-    ])
-  }
-
   fn hidden(path: &Path) -> Result<bool, Error> {
     use std::os::windows::fs::MetadataExt;
 
@@ -25,10 +16,6 @@ impl PlatformInterface for Platform {
 
 #[cfg(target_os = "macos")]
 impl PlatformInterface for Platform {
-  fn opener() -> Result<Vec<OsString>, Error> {
-    Ok(vec![OsString::from("open")])
-  }
-
   fn hidden(path: &Path) -> Result<bool, Error> {
     use std::{ffi::CString, mem, os::unix::ffi::OsStrExt};
 
@@ -53,20 +40,6 @@ impl PlatformInterface for Platform {
 
 #[cfg(not(any(target_os = "windows", target_os = "macos")))]
 impl PlatformInterface for Platform {
-  fn opener() -> Result<Vec<OsString>, Error> {
-    const OPENERS: &[&str] = &["xdg-open", "gnome-open", "kde-open"];
-
-    for opener in OPENERS {
-      if let Ok(output) = Command::new(opener).arg("--version").output() {
-        if output.status.success() {
-          return Ok(vec![OsString::from(opener)]);
-        }
-      }
-    }
-
-    Err(Error::OpenerMissing { tried: OPENERS })
-  }
-
   fn hidden(_path: &Path) -> Result<bool, Error> {
     Ok(false)
   }

src/platform_interface.rs

@@ -2,49 +2,22 @@ use crate::common::*;
 
 pub(crate) trait PlatformInterface {
   fn open_file(path: &Path) -> Result<(), Error> {
-    Self::open_raw(path.as_os_str())
+    Self::open_target(path.as_ref())
   }
 
   fn open_url(url: &Url) -> Result<(), Error> {
-    if cfg!(windows) {
-      let escaped = format!("\"{}\"", url);
-      Self::open_raw(escaped.as_str().as_ref())
-    } else {
-      Self::open_raw(url.as_str().as_ref())
-    }
+    Self::open_target(url.as_str().as_ref())
   }
 
-  fn open_raw(target: &OsStr) -> Result<(), Error> {
-    let mut command = Self::opener()?;
-    command.push(OsString::from(target));
+  fn open_target(target: &OsStr) -> Result<(), Error> {
+    let exit_status = open::that(target).context(error::OpenerInvoke)?;
 
-    let command_string = || {
-      command
-        .iter()
-        .map(|arg| arg.to_string_lossy().into_owned())
-        .collect::<Vec<String>>()
-        .join(",")
-    };
+    if !exit_status.success() {
+      return Err(Error::OpenerExitStatus { exit_status });
+    }
 
-    let status = Command::new(&command[0])
-      .args(&command[1..])
-      .status()
-      .map_err(|source| Error::CommandInvoke {
-        source,
-        command: command_string(),
-      })?;
-
-    if status.success() {
     Ok(())
-    } else {
-      Err(Error::CommandStatus {
-        command: command_string(),
-        status,
-      })
   }
-  }
-
-  fn opener() -> Result<Vec<OsString>, Error>;
 
   fn hidden(path: &Path) -> Result<bool, Error>;
 }

src/subcommand/torrent/create.rs

@@ -1352,69 +1352,6 @@ mod tests {
     }
   }
 
-  #[test]
-  fn open() {
-    let mut env = test_env! {
-      args: [
-        "torrent",
-        "create",
-        "--input",
-        "foo",
-        "--announce",
-        "http://bar",
-        "--open",
-      ],
-      tree: {},
-    };
-
-    let opened = env.resolve("opened.txt");
-    let torrent = env.resolve("foo.torrent");
-
-    let expected = if cfg!(target_os = "windows") {
-      let script = env.resolve("open.bat");
-      fs::write(&script, format!("echo %3 > {}", opened.display())).unwrap();
-      format!("{} \r\n", torrent.display())
-    } else {
-      let script = env.resolve(&Platform::opener().unwrap()[0]);
-      fs::write(
-        &script,
-        format!("#!/usr/bin/env sh\necho $1 > {}", opened.display()),
-      )
-      .unwrap();
-
-      Command::new("chmod")
-        .arg("+x")
-        .arg(&script)
-        .status()
-        .unwrap();
-
-      format!("{}\n", torrent.display())
-    };
-
-    const KEY: &str = "PATH";
-    let path = env::var_os(KEY).unwrap();
-    let mut split = env::split_paths(&path)
-      .into_iter()
-      .collect::<Vec<PathBuf>>();
-    split.insert(0, env.dir().to_owned());
-    let new = env::join_paths(split).unwrap();
-    env::set_var(KEY, new);
-
-    fs::write(env.resolve("foo"), "").unwrap();
-    env.run().unwrap();
-
-    let start = Instant::now();
-
-    while start.elapsed() < Duration::new(2, 0) {
-      if let Ok(text) = fs::read_to_string(&opened) {
-        assert_eq!(text, expected);
-        return;
-      }
-    }
-
-    panic!("Failed to read `opened.txt`.");
-  }
-
   #[test]
   fn uneven_piece_length() {
     let mut env = test_env! {

src/subcommand/torrent/link.rs

@@ -239,81 +239,4 @@ mod tests {
       if input == "foo.torrent"
     );
   }
-
-  #[test]
-  fn open() {
-    let mut create_env = test_env! {
-      args: [
-        "torrent",
-        "create",
-        "--input",
-        "foo",
-      ],
-      tree: {
-        foo: "",
-      },
-    };
-
-    assert_matches!(create_env.run(), Ok(()));
-
-    let torrent = create_env.resolve("foo.torrent");
-
-    let mut env = test_env! {
-      args: [
-        "torrent",
-        "link",
-        "--input",
-        &torrent,
-        "--open",
-      ],
-      tree: {},
-    };
-
-    let opened = env.resolve("opened.txt");
-
-    let link = "magnet:?xt=urn:btih:516735f4b80f2b5487eed5f226075bdcde33a54e&dn=foo";
-
-    let expected = if cfg!(target_os = "windows") {
-      let script = env.resolve("open.bat");
-      fs::write(&script, format!("echo > {}", opened.display())).unwrap();
-      format!("ECHO is on.\r\n")
-    } else {
-      let script = env.resolve(&Platform::opener().unwrap()[0]);
-      fs::write(
-        &script,
-        format!("#!/usr/bin/env sh\necho $1 > {}", opened.display()),
-      )
-      .unwrap();
-
-      Command::new("chmod")
-        .arg("+x")
-        .arg(&script)
-        .status()
-        .unwrap();
-
-      format!("{}\n", link)
-    };
-
-    const KEY: &str = "PATH";
-    let path = env::var_os(KEY).unwrap();
-    let mut split = env::split_paths(&path)
-      .into_iter()
-      .collect::<Vec<PathBuf>>();
-    split.insert(0, env.dir().to_owned());
-    let new = env::join_paths(split).unwrap();
-    env::set_var(KEY, new);
-
-    assert_matches!(env.run(), Ok(()));
-
-    let start = Instant::now();
-
-    while start.elapsed() < Duration::new(2, 0) {
-      if let Ok(text) = fs::read_to_string(&opened) {
-        assert_eq!(text, expected);
-        return;
-      }
-    }
-
-    panic!("Failed to read `opened.txt`.");
-  }
 }