From 0147dae9fe2a863105596ae81c61078bde6cb8af Mon Sep 17 00:00:00 2001
From: Dan Wilhelm <dan@danwilhelm.com>
Date: Thu, 10 Jan 2019 23:10:13 -0800
Subject: [PATCH 1/2] login: Fix empty input prompt

Issue #484.

PR392 inadvertantly replaced the `login` interactive process spawner
with
`child::run`, which is hard-coded to buffer stdout/stderr. This caused
`login` to become essentially unusable; the user could no longer see
interactive input prompts or error messages displayed by `npm adduser`.

The code was not directly reverted because the previous version:
1. Returned Error instead of failure::Error. (Updated to use
`bail!`, which is consistent with `publish`.)
2. Displayed all stderr only upon exit, rather than interactively
displaying it. This led to repeated interactive prompts without
informing the user why. (Updated to use `status()` which inherits
stdin/stdout/stderr by default.)
3. Did not provide logging. (Now duplicates the logging in
`child::run`.)
---
 src/npm.rs | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/npm.rs b/src/npm.rs
index 76fe841..4f7b18a 100644
--- a/src/npm.rs
+++ b/src/npm.rs
@@ -62,12 +62,14 @@ pub fn npm_login(
         args.push_str(&format!(" --auth_type={}", auth_type));
     }
 
+    // Interactively ask user for npm login info.
+    //  (child::run does not support interactive input)
     let mut cmd = Command::new("npm");
-    cmd.arg("login")
-        .arg(args)
-        .stdin(Stdio::inherit())
-        .stdout(Stdio::inherit());
-    child::run(log, cmd, "npm login")
-        .with_context(|_| format!("Login to registry {} failed", registry))?;
-    Ok(())
+    cmd.arg("login").arg(args);
+
+    info!(log, "Running {:?}", cmd);
+    match cmd.status()?.success() {
+        true => Ok(()),
+        false => bail!("Login to registry {} failed", registry),
+    }
 }

From 2343e97e1ec3e622c73e6e70ebbd5b6374612f04 Mon Sep 17 00:00:00 2001
From: Dan Wilhelm <dan@danwilhelm.com>
Date: Thu, 10 Jan 2019 23:43:16 -0800
Subject: [PATCH 2/2] login: Send arguments separately to npm

All arguments were passed to `arg` inside one string when building a
`Command`. However, using the `arg` function, "only one argument can be
passed per use." This caused all arguments accidentally to be appended
to the registry URL. For example: After a successful login with a
provided `--auth_type`, the success message incorrectly displayed:
"Logged in as asf on https://registry.npmjs.org/%20--auth_type=Basic."
The space (%20 in hex) was caused by adding a fixed space before each
additional argument.

This commit pushes all arguments onto a `Vec<String>`. Then, the `args`
function adds the arguments separately to the command. This removes the
need to prepend spaces to each argument. Alternatively, `arg` could have
been used throughout to build the command argument-by-argument. However,
using `args` partitions the code more neatly into two distinct sections.
---
 src/npm.rs | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/npm.rs b/src/npm.rs
index 4f7b18a..6b09e63 100644
--- a/src/npm.rs
+++ b/src/npm.rs
@@ -46,26 +46,24 @@ pub fn npm_login(
     always_auth: bool,
     auth_type: &Option<String>,
 ) -> Result<(), failure::Error> {
-    let mut args = String::new();
-
-    args.push_str(&format!("--registry={}", registry));
+    let mut args = vec![format!("login"), format!("--registry={}", registry)];
 
     if let Some(scope) = scope {
-        args.push_str(&format!(" --scope={}", scope));
+        args.push(format!("--scope={}", scope));
     }
 
     if always_auth == true {
-        args.push_str(" --always_auth");
+        args.push(format!("--always_auth"));
     }
 
     if let Some(auth_type) = auth_type {
-        args.push_str(&format!(" --auth_type={}", auth_type));
+        args.push(format!("--auth_type={}", auth_type));
     }
 
     // Interactively ask user for npm login info.
     //  (child::run does not support interactive input)
     let mut cmd = Command::new("npm");
-    cmd.arg("login").arg(args);
+    cmd.args(args);
 
     info!(log, "Running {:?}", cmd);
     match cmd.status()?.success() {