config: simplify regex

[caub]

explosive quantifiers can be nasty, that one was probably ok, but I think it's better to be more restrictive

[coveralls]

Coverage remained the same at 100.0% when pulling 423756c on caub:patch-1 into 1945a05 on motdotla:master.

[zenflow]

This optimizes for performance over simplicity, no? Just pointing out that this doesn't "simplify" as the title says

[caub]

@zenflow yes performance (or rather to avoid a possibly very long regexp), but I realize it's not that much faster

process.argv.slice(1). is useful at least, then it depends if you prefer to keep the regexp or the split version, they seem to perform comparably on small inputs

var data = Array.from({length: 4e5}, () => `dotenv_config${Math.random()<.3?'_':'.'}foobar-qux${Math.floor(1e16*Math.random()).toString(36)}${Math.random()<.7?'=':'.'}qu_ux_xu${Math.floor(1e16*Math.random()).toString(36)}`)

var re1 = () => {
  data.forEach(val => {
    const m = val.match(/^dotenv_config_(.+)=(.+)/);
  });
};
var re2 =  () => {
  data.forEach(val => {
    const m = val.match(/^dotenv_config_([^=]+)=(.+)$/);
  })
};
var slice =  () => {
  data.forEach(val => {
    const m = val.slice(14).split('=', 2);
  })
};
bench(re1)
bench(re1)
bench(re2)
bench(re2)
bench(slice)
bench(slice)

bench(re1)
bench(re1)
bench(re2)
bench(re2)
bench(slice)
bench(slice)

function bench(fn) {
  console.time(fn.name);
  fn();
  console.timeEnd(fn.name);
}

[maxbeatty]

Thanks for providing a benchmark to lay out the performance improvements. I'm not sure we should be optimizing for the use case of someone providing 400,000 command line arguments.

I created a test on jsPerf using the newer option matcher from #304 and what I expect most people to be feeding process.argv. Obviously a browser-based benchmark isn't apples to apples with a Node-based benchmark, but with V8 under the hood for both, I'm content.

https://jsperf.com/dotenv-cli-parsing/1

I'll leave this open for a bit so people can run the jsPerf test. I'm see the #304 implementation being ~40% faster. If that's the consensus, we can leave it as-is.

[caub]

Here's a bench on node: https://repl.it/@caub/dotenv-argv-parse (not a huge fan of jsperf, accurate benchmarks can be done with console.time or perf_hooks)

I didn't even try to get it fast (.slice(1).forEach is surely 2x at least slower than a for loop)

Just trying to avoid .* quantifiers (http://www.rexegg.com/regex-explosive-quantifiers.html)

So indeed, regexp is faster, more readable too, but my HEAD^ commit version, let me revert then

[caub]

again, not specially optimized for perf, it really doesn't matter at that scale, it's just more readable I think

[zenflow]

I like this new version much better because it doesn't sacrifice any simplicity/readability

[zenflow]

@caub thanks for the reference on explosive quantifiers!

[maxbeatty]

Will you update this or open a new PR against master now that this code has moved?

[maxbeatty]

this PR could be simplified to:

-const re = /^dotenv_config_(.+)=(.+)/
+const re = /^dotenv_config_([^=]+)=(.+)$/

additional tests are always welcomed

[maxbeatty]

closing in favor of #325

[caub]

@maxbeatty ok, you might want that .slice(1) there

dotenv/config.js

Line 3 in bfb2269

require('./lib/cli-options')(process.argv)

since first item is the node ccommand