Add command line parameters to ignore the local .bin folder and emit verbose output

[tatablack]

This effectively restores the code reverted with #26, while fixing the underlying issue (which comes from the commander library, see tj/commander.js#512).

For context, the new functionality which will be (re)introduced is:

• new command line parameters:

  • -v: emits verbose output
  • -i: ignores binaries in the project's local bin folder, thus avoiding errors like the following:

  Expected npm version to match ^6.4.1, but got 5.6.0

  ..even when the npm version is correct. This is due to npx prepending the local ./node_modules/.bin folder to the PATH environment variable, mirroring npm's behaviour.

• update README with info about the two new command line parameters

• small refactoring in Rollup's configuration file

[georgegillams]

Looks good. Just a couple of things I've seen from my initial pass

[georgegillams]

Overriding the behaviour of console.log feels very fragile.
Instead, could we create a function called verboseLog which can log if verbose mode is on, and otherwise just return?

As well as being less hacky, I think it would also be less confusing to future contributors

[tatablack]

I wouldn't say it's fragile (as in, it wouldn't be prone to breaking), but I agree it may not be immediately obvious to contributors what's the proper way to log things.

What about creating a global logger? That may make it clearer that logging levels should be respected (while console usage is arguably less strict). Something like:

const logger = Object.create(console, {
  debug: {
    value: (message, ...args) => {
      if (userInput.verbose) {
        console.debug(message, ...args);
      }
    }
});

Or, to be honest, it could be as simple as the current overriding behaviour, but with the logger aliasing to make it clearer throughout the code:

const logger = console;
if (!userInput.verbose) {
  logger.debug = () => ();
}

[tatablack]

I tried implementing option 2, let me know if you want me to push it to see how it looks.

[tatablack]

Pushed, both the console → logger replacement and the suggestion about --verbose mentioned below. Waiting for feedback. 🙂

[matthewdavidson]

@georgegillams,

Overriding the behaviour of console.log feels very fragile.

Noted, but this is a small cli tool with one source code file. I think it's manageable. If it were a bigger codebase then I'd tend to agree.

Assigning console to logger just introduces even more confusion IMO. const logger = console; is simply assigning the reference to a new variable and the act of reassigning logger.log is still modifying console.

If anything we should do this:

const logger = {
  debug: (...args) => {
      if (verbose) {
        console.debug(...args);
      }
  },
  ...
};

[matthewdavidson]

I've added a commit doing something along these lines ...

[georgegillams]

LGTM - You'll need to rebuild this and commit the changes in dist too 👍

[georgegillams]

Also tested locally - works really well 🎉

[tatablack]

Updated and pushed build artifacts.

[matthewdavidson]

I added a commit that removes commander. This has 2 benefits:

• we avoid the dodgy code for working around the argv issue in commander
• we avoid an unnecessary extra dep

Test the previously unforeseen issue like so:

node -e "$(curl -fsSL https://raw.githubusercontent.com/tatablack/ensure-node-env/85cc2363ea6b10ca0d04e6b59a629a329ebbb4a6/dist/index.js)"