Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Create helper function to parse global options #1955

Merged
merged 5 commits into from
Mar 26, 2019
Merged

Conversation

grassias
Copy link
Contributor

No description provided.

@grassias
Copy link
Contributor Author

This PR aims to address issue #1947 after @alanshaw suggestions.

@hugomrdias
Copy link
Member

i propose to do something like this

  let getIPFS = null
  cli
    .commandDir('commands')
    .middleware(argv => {
      getIPFS = argv.getIpfs = utils.singleton(cb => utils.getIPFS(argv, cb))
      return argv
    })
    .help()
    .strict()
    .completion()
  let exitCode = 0

  try {
    const { data } = await new YargsPromise(cli).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')
    }

    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
      }
    }
  }

  if (exitCode) {
    process.exit(exitCode)
  }
}

this way we use only one yargs instance

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test please?

@grassias
Copy link
Contributor Author

I'd like so but I'm not sure yet about how to handle it. Is there any test utility/method that exposes the instantiated IPFS object produced by cmds?

@hugomrdias
Copy link
Member

to do this we need to change the way we test the cli check the example below

https://codesandbox.io/s/mqq66j9v6x

@hugomrdias
Copy link
Member

i improved the example to further simplify this code, can you have a look?
its best to have a separate parser.js file with all the parser logic so we can test all the cmds in this way.

can you also make one more test with a real cmd like id and assert an getIpfs.instance exists ?

thank you so much for your contribution 👍

@alanshaw alanshaw merged commit 1c07779 into ipfs:master Mar 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants