fix: CLI parsing of --silent arg (#1955)

* fix: create helper function to parse global options

* fix: use only one yargs instance

* fix: export yargs parser and add some tests

* fix: separate yargs parser for ease of testing

* fix: remove unneeded dependency

fixes #1947

Author: grassias

src/cli/bin.js

@@ -3,91 +3,56 @@
 'use strict'
 
 const YargsPromise = require('yargs-promise')
-const yargs = require('yargs/yargs')
 const updateNotifier = require('update-notifier')
 const utils = require('./utils')
 const print = utils.print
 const mfs = require('ipfs-mfs/cli')
 const debug = require('debug')('ipfs:cli')
 const pkg = require('../../package.json')
+const parser = require('./parser')
 
 async function main (args) {
   const oneWeek = 1000 * 60 * 60 * 24 * 7
   updateNotifier({ pkg, updateCheckInterval: oneWeek }).notify()
 
-  const cli = yargs(args)
-    .option('silent', {
-      desc: 'Write no output',
-      type: 'boolean',
-      default: false,
-      coerce: silent => {
-        if (silent) utils.disablePrinting()
-        return silent
-      }
-    })
-    .option('pass', {
-      desc: 'Pass phrase for the keys',
-      type: 'string',
-      default: ''
-    })
-    .epilog(utils.ipfsPathHelp)
-    .demandCommand(1)
-    .fail((msg, err, yargs) => {
-      if (err) {
-        throw err // preserve stack
-      }
-
-      if (args.length > 0) {
-        print(msg)
-      }
-
-      yargs.showHelp()
-    })
-
-  // Function to get hold of a singleton ipfs instance
-  const getIpfs = utils.singleton(cb => utils.getIPFS(yargs(args).argv, cb))
+  const cli = new YargsPromise(parser)
 
   // add MFS (Files API) commands
   mfs(cli)
 
-  cli
-    .commandDir('commands')
-    .help()
-    .strict()
-    .completion()
-
-  let exitCode = 0
-
-  try {
-    const { data } = await new YargsPromise(cli, { getIpfs }).parse(args)
-    if (data) print(data)
-  } catch (err) {
-    debug(err)
-
-    // the argument can have a different shape depending on where the error came from
-    if (err.message || (err.error && err.error.message)) {
-      print(err.message || err.error.message)
-    } else {
-      print('Unknown error, please re-run the command with DEBUG=ipfs:cli to see debug output')
-    }
+  let getIpfs = null
 
-    exitCode = 1
-  } finally {
-    // If an IPFS instance was used in the handler then clean it up here
-    if (getIpfs.instance) {
-      try {
-        const cleanup = getIpfs.rest[0]
-        await cleanup()
-      } catch (err) {
-        debug(err)
-        exitCode = 1
+  cli
+    .parse(args)
+    .then(({ data, argv }) => {
+      getIpfs = argv.getIpfs
+      if (data) {
+        print(data)
       }
-    }
-  }
-
-  if (exitCode) {
-    process.exit(exitCode)
-  }
+    })
+    .catch(({ error, argv }) => {
+      getIpfs = argv.getIpfs
+      debug(error)
+      // the argument can have a different shape depending on where the error came from
+      if (error.message || (error.error && error.error.message)) {
+        print(error.message || error.error.message)
+      } else {
+        print('Unknown error, please re-run the command with DEBUG=ipfs:cli to see debug output')
+      }
+      process.exit(1)
+    })
+    .finally(async () => {
+      // If an IPFS instance was used in the handler then clean it up here
+      if (getIpfs && getIpfs.instance) {
+        try {
+          const cleanup = getIpfs.rest[0]
+          await cleanup()
+        } catch (err) {
+          debug(err)
+          process.exit(1)
+        }
+      }
+    })
 }
 
 main(process.argv.slice(2))

src/cli/parser.js

@@ -0,0 +1,41 @@
+'use strict'
+
+const yargs = require('yargs')
+const utils = require('./utils')
+const print = utils.print
+
+const parser = yargs
+  .option('silent', {
+    desc: 'Write no output',
+    type: 'boolean',
+    default: false,
+    coerce: silent => {
+      if (silent) utils.disablePrinting()
+      return silent
+    }
+  })
+  .option('pass', {
+    desc: 'Pass phrase for the keys',
+    type: 'string',
+    default: ''
+  })
+  .epilog(utils.ipfsPathHelp)
+  .demandCommand(1)
+  .fail((msg, err, yargs) => {
+    if (err) {
+      throw err // preserve stack
+    }
+    print(msg)
+    yargs.showHelp()
+  })
+  .commandDir('commands')
+  .middleware(argv => {
+    // Function to get hold of a singleton ipfs instance
+    argv.getIpfs = utils.singleton(cb => utils.getIPFS(argv, cb))
+    return argv
+  })
+  .help()
+  .strict()
+  .completion()
+
+module.exports = parser

test/cli/parser.js

@@ -0,0 +1,45 @@
+/* eslint-env mocha */
+'use strict'
+
+const chai = require('chai')
+const dirtyChai = require('dirty-chai')
+const expect = chai.expect
+chai.use(dirtyChai)
+const parser = require('../../src/cli/parser')
+const YargsPromise = require('yargs-promise')
+
+describe('yargs cli parser', () => {
+  let cli
+
+  before(() => {
+    cli = new YargsPromise(parser)
+  })
+
+  it('should handle --silent flag correctly', (done) => {
+    cli
+      .parse('serve --silent src/init-files/init-docs/readme')
+      .then(({ error, argv }) => {
+        expect(error).to.not.exist()
+        expect(argv).to.include({ silent: true, pass: '' })
+        expect(argv.getIpfs.instance).to.exist()
+        done()
+      })
+      .catch(({ error }) => {
+        done(error)
+      })
+  })
+
+  it('should handle --pass flag correctly', (done) => {
+    cli
+      .parse('serve --pass password')
+      .then(({ error, argv }) => {
+        expect(error).to.not.exist()
+        expect(argv).to.include({ silent: true, pass: '' })
+        expect(argv.getIpfs.instance).to.exist()
+        done()
+      })
+      .catch(({ error }) => {
+        done(error)
+      })
+  })
+})