From 45dc52886e485a747e3b29f72d9cd7584c470067 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Astori?= <jeremie@astori.fr>
Date: Sat, 13 Jan 2018 02:00:37 -0500
Subject: [PATCH] Fix `thelounge uninstall` command

- Exit with code 1 when package was not installed
- Run a pre-step to check if package is installed before uninstalling. I have tried every possible way I could think of without that first `npm list` step based on output of `npm uninstall`, but different npm versions produce different outputs, so there is nothing reliable. This is a more robust way to do so anyway.
- Consolidate error handlers
---
 src/command-line/uninstall.js | 84 ++++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 27 deletions(-)

diff --git a/src/command-line/uninstall.js b/src/command-line/uninstall.js
index 71c63538..be797f88 100644
--- a/src/command-line/uninstall.js
+++ b/src/command-line/uninstall.js
@@ -28,45 +28,75 @@ program
 
 		if (!fs.existsSync(packagesConfig)) {
 			log.warn(packageWasNotInstalled);
-			return;
+			process.exit(1);
 		}
 
-		const npm = child.spawn(
-			process.platform === "win32" ? "npm.cmd" : "npm",
+		const npm = process.platform === "win32" ? "npm.cmd" : "npm";
+		const errorHandler = (error) => {
+			log.error(
+				`Failed to uninstall ${colors.green(packageName)}. ` +
+				`${typeof x === "number" ? "Exit code" : "Error"}: ${error}`
+			);
+			process.exit(1);
+		};
+
+		// First, we check if the package is installed with `npm list`
+		const list = child.spawn(
+			npm,
 			[
-				"uninstall",
-				"--no-progress",
+				"list",
+				"--depth",
+				"0",
 				"--prefix",
 				packagesParent,
 				packageName,
 			],
 			{
-				// This is the same as `"inherit"` except `process.stdout` is piped
-				stdio: [process.stdin, "pipe", process.stderr],
+				// This is the same as `"inherit"` except:
+				// - `process.stdout` is piped so we can test if the output mentions the
+				//   package was found
+				// - `process.stderr` is ignored to silence `npm ERR! extraneous` errors
+				stdio: [process.stdin, "pipe", "ignore"],
 			}
 		);
 
-		let hasUninstalled = false;
-
-		npm.stdout.on("data", () => {
-			hasUninstalled = true;
-		});
-
-		npm.on("error", (e) => {
-			log.error(`${e}`);
-			process.exit(1);
-		});
-
-		npm.on("close", (code) => {
-			if (code !== 0) {
-				log.error(`Failed to uninstall ${colors.green(packageName)}. Exit code: ${code}`);
-				return;
-			}
-
-			if (hasUninstalled) {
-				log.info(`${colors.green(packageName)} has been successfully uninstalled.`);
-			} else {
+		list.stdout.on("data", (data) => {
+			// If the package name does not appear in stdout, it means it was not
+			// installed. We cannot rely on exit code because `npm ERR! extraneous`
+			// causes a status of 1 even if package exists.
+			if (!data.toString().includes(packageName)) {
 				log.warn(packageWasNotInstalled);
+				process.exit(1);
 			}
 		});
+
+		list.on("error", errorHandler);
+
+		list.on("close", () => {
+			// If we get there, it means the package exists, so uninstall
+			const uninstall = child.spawn(
+				npm,
+				[
+					"uninstall",
+					"--no-progress",
+					"--prefix",
+					packagesParent,
+					packageName,
+				],
+				{
+					// This is the same as `"inherit"` except `process.stdout` is silenced
+					stdio: [process.stdin, "ignore", process.stderr],
+				}
+			);
+
+			uninstall.on("error", errorHandler);
+
+			uninstall.on("close", (code) => {
+				if (code !== 0) {
+					errorHandler(code);
+				}
+
+				log.info(`${colors.green(packageName)} has been successfully uninstalled.`);
+			});
+		});
 	});