[cli]: Consistent list logging (#4287)

* add consistent logger

* improve log logic

cli/src/commands/install.js

@@ -36,6 +36,7 @@ import {
 } from '../lib/npm/npmProjectUtils';
 import {getRangeLowerBound} from '../lib/semver';
 import {createStub, pkgHasFlowFiles} from '../lib/stubUtils';
+import {listItem} from '../lib/logger';
 
 export const name = 'install [explicitLibDefs...]';
 export const description = 'Installs libdefs into the ./flow-typed directory';
@@ -273,15 +274,7 @@ async function installEnvLibDefs(
               const localFile = fs.readFileSync(defLocalPath, 'utf-8');
 
               if (!verifySignedCode(localFile) && !overwrite) {
-                console.log(
-                  colors.bold(
-                    '  • %s\n' +
-                      '    └> %s\n' +
-                      '       %s\n' +
-                      '       %s\n' +
-                      '       %s\n' +
-                      '       %s',
-                  ),
+                listItem(
                   en,
                   colors.red(
                     `${en} already exists and appears to have been manually written or changed!`,
@@ -312,16 +305,14 @@ async function installEnvLibDefs(
             const codeSignPreprocessor = signCodeStream(repoVersion);
             await copyFile(def.path, defLocalPath, codeSignPreprocessor);
 
-            console.log(
-              colors.bold('  • %s\n' + '    └> %s'),
+            listItem(
               en,
               colors.green(
                 `.${path.sep}${path.relative(flowProjectRoot, defLocalPath)}`,
               ),
             );
           } else {
-            console.log(
-              colors.bold('  • %s\n' + '    └> %s'),
+            listItem(
               en,
               colors.yellow(
                 `Was unable to install ${en}. The env might not exist or there is not a version compatible with your version of flow`,
@@ -703,8 +694,7 @@ async function installNpmLibDef(
     );
 
     if (!overwrite && (await fs.exists(filePath))) {
-      console.error(
-        '  • %s\n' + '    %s\n    %s\n    └> %s',
+      listItem(
         colors.bold(
           colors.red(
             `${terseFilePath} already exists and appears to have been manually ` +
@@ -727,11 +717,7 @@ async function installNpmLibDef(
     const codeSignPreprocessor = signCodeStream(repoVersion);
     await copyFile(npmLibDef.path, filePath, codeSignPreprocessor);
 
-    console.log(
-      colors.bold('  • %s\n' + '    └> %s'),
-      fileName,
-      colors.green(`.${path.sep}${terseFilePath}`),
-    );
+    listItem(fileName, colors.green(`.${path.sep}${terseFilePath}`));
 
     // Remove any lingering stubs
     console.log(npmLibDef.name);

cli/src/lib/__tests__/logger.js

@@ -0,0 +1,58 @@
+// @flow
+import colors from 'colors';
+
+import {listItem} from '../logger';
+
+describe('logger', () => {
+  const consoleLog = jest.fn();
+  jest.spyOn(console, 'log').mockImplementation(consoleLog);
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  describe('listItem', () => {
+    it('logs single value', () => {
+      listItem('test');
+
+      expect(consoleLog).toHaveBeenCalledWith(colors.bold(`  • test`));
+    });
+
+    it('logs two values', () => {
+      listItem('test', 'a');
+
+      expect(consoleLog).toHaveBeenCalledWith(
+        colors.bold(`  • test
+    └> a`),
+      );
+    });
+
+    it('logs multiple values', () => {
+      listItem('test', 'a', 'b');
+
+      expect(consoleLog).toHaveBeenCalledWith(
+        colors.bold(`  • test
+    └> a
+       b`),
+      );
+    });
+
+    it('ignores undefined values', () => {
+      listItem(undefined, 'a', 'b');
+
+      expect(consoleLog).toHaveBeenCalledWith(
+        colors.bold(`  • a
+    └> b`),
+      );
+    });
+
+    it('ignores empty strings', () => {
+      listItem('test', '', 'b');
+
+      expect(consoleLog).toHaveBeenCalledWith(
+        colors.bold(`  • test
+    └> b`),
+      );
+    });
+  });
+});

cli/src/lib/logger.js

@@ -0,0 +1,32 @@
+// @flow
+import colors from 'colors';
+
+const bulletPoint = `  • `;
+const arrow = `    └> `;
+const genericRowPad = `       `;
+
+const shouldNewLine = (condition: boolean) => (condition ? '\n' : '');
+
+const createListItem = (values: Array<string>): string => {
+  const [first, second, ...rest] = values;
+  let value = `${bulletPoint}${first}${shouldNewLine(!!second)}`;
+  if (second) {
+    value += `${arrow}${second}${shouldNewLine(rest.length > 0)}`;
+  }
+  if (rest.length > 0) {
+    rest.forEach((r, i) => {
+      value += `${genericRowPad}${r}${shouldNewLine(i !== rest.length - 1)}`;
+    });
+  }
+  return colors.bold(value);
+};
+
+export const listItem = (...values: Array<string | void>): void => {
+  const validItems: Array<string> = values
+    .map(o => {
+      if (!o) return '';
+      return o;
+    })
+    .filter(o => !!o);
+  console.log(createListItem(validItems));
+};

cli/src/lib/stubUtils.js

@@ -19,6 +19,7 @@ import {path} from './node';
 import {signCode} from './codeSign';
 import {verifySignedCode} from './codeSign';
 import {versionToString} from './semver';
+import {listItem} from './logger';
 
 export function glob(pattern: string, options: Object): Promise<Array<string>> {
   return new Promise((resolve, reject) =>
@@ -532,22 +533,15 @@ export async function createStub(
       typescriptTypingsContent,
     );
     const terseFilename = path.relative(projectRoot, filename);
-    console.log(
-      colors.bold('  • %s@%s\n' + '    └> %s'),
-      packageName,
-      version,
+    listItem(
+      `${packageName}@${version}`,
       colors.red(terseFilename),
+      resolutionError
+        ? colors.yellow(
+            `Unable to stub all files in ${packageName}, so only created a stub for the main module (${resolutionError.message})`,
+          )
+        : undefined,
     );
-    if (resolutionError) {
-      console.log(
-        colors.yellow(
-          "\t  Unable to stub all files in '%s', " +
-            'so only created a stub for the main module (%s)',
-        ),
-        packageName,
-        resolutionError.message,
-      );
-    }
     return true;
   } catch (e) {
     console.log(