diff options
author | Anna Henningsen <anna@addaleax.net> | 2018-09-22 14:34:00 +0200 |
---|---|---|
committer | Daniel Bevenius <daniel.bevenius@gmail.com> | 2018-09-25 13:32:30 +0200 |
commit | 0227635315c3aa1c31e6814325822a1e4306372e (patch) | |
tree | c44ead0ca5f44ec7b17a8f9830da63bce06d3d00 | |
parent | 5605cec0db9481d4cc85da1a7869f1d725c4357f (diff) | |
download | android-node-v8-0227635315c3aa1c31e6814325822a1e4306372e.tar.gz android-node-v8-0227635315c3aa1c31e6814325822a1e4306372e.tar.bz2 android-node-v8-0227635315c3aa1c31e6814325822a1e4306372e.zip |
cli: normalize `_` → `-` when parsing options
This allows for option syntax similar to V8’s one, e.g.
`--no_warnings` has the same effect as `--no-warnings`.
PR-URL: https://github.com/nodejs/node/pull/23020
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
-rw-r--r-- | doc/api/cli.md | 17 | ||||
-rw-r--r-- | lib/internal/bootstrap/node.js | 43 | ||||
-rw-r--r-- | src/node_options-inl.h | 19 | ||||
-rw-r--r-- | src/node_options.cc | 12 | ||||
-rw-r--r-- | test/parallel/test-cli-node-options-disallowed.js | 1 | ||||
-rw-r--r-- | test/parallel/test-cli-node-options.js | 1 | ||||
-rw-r--r-- | test/parallel/test-pending-deprecation.js | 8 | ||||
-rw-r--r-- | test/parallel/test-process-env-allowed-flags.js | 2 |
8 files changed, 45 insertions, 58 deletions
diff --git a/doc/api/cli.md b/doc/api/cli.md index 3f5441b82e..9c6195aa4e 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -21,6 +21,18 @@ Execute without arguments to start the [REPL][]. _For more info about `node inspect`, please see the [debugger][] documentation._ ## Options +<!-- YAML +changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/23020 + description: Underscores instead of dashes are now allowed for + Node.js options as well, in addition to V8 options. +--> + +All options, including V8 options, allow words to be separated by both +dashes (`-`) or underscores (`_`). + +For example, `--pending-deprecation` is equivalent to `--pending_deprecation`. ### `-` <!-- YAML @@ -390,11 +402,6 @@ added: v0.1.3 Print V8 command line options. -V8 options allow words to be separated by both dashes (`-`) or -underscores (`_`). - -For example, `--stack-trace-limit` is equivalent to `--stack_trace_limit`. - ### `--v8-pool-size=num` <!-- YAML added: v5.10.0 diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index bcbb96a87f..65986337c3 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -742,7 +742,7 @@ // This builds process.allowedNodeEnvironmentFlags // from data in the config binding - const replaceDashesRegex = /-/g; + const replaceUnderscoresRegex = /_/g; const leadingDashesRegex = /^--?/; const trailingValuesRegex = /=.*$/; @@ -754,20 +754,14 @@ const get = () => { const { getOptions, - types: { kV8Option }, envSettings: { kAllowedInEnvironment } } = internalBinding('options'); const { options, aliases } = getOptions(); - const allowedV8EnvironmentFlags = []; const allowedNodeEnvironmentFlags = []; for (const [name, info] of options) { if (info.envVarSettings === kAllowedInEnvironment) { - if (info.type === kV8Option) { - allowedV8EnvironmentFlags.push(name); - } else { - allowedNodeEnvironmentFlags.push(name); - } + allowedNodeEnvironmentFlags.push(name); } } @@ -801,11 +795,9 @@ // process.allowedNodeEnvironmentFlags.has() which lack leading dashes. // Avoid interference w/ user code by flattening `Set.prototype` into // each object. - const [nodeFlags, v8Flags] = [ - allowedNodeEnvironmentFlags, allowedV8EnvironmentFlags - ].map((flags) => Object.defineProperties( - new Set(flags.map(trimLeadingDashes)), - Object.getOwnPropertyDescriptors(Set.prototype)) + const nodeFlags = Object.defineProperties( + new Set(allowedNodeEnvironmentFlags.map(trimLeadingDashes)), + Object.getOwnPropertyDescriptors(Set.prototype) ); class NodeEnvironmentFlagsSet extends Set { @@ -829,29 +821,18 @@ has(key) { // This will return `true` based on various possible // permutations of a flag, including present/missing leading - // dash(es) and/or underscores-for-dashes in the case of V8-specific - // flags. Strips any values after `=`, inclusive. + // dash(es) and/or underscores-for-dashes. + // Strips any values after `=`, inclusive. // TODO(addaleax): It might be more flexible to run the option parser // on a dummy option set and see whether it rejects the argument or // not. if (typeof key === 'string') { - key = replace(key, trailingValuesRegex, ''); + key = replace(key, replaceUnderscoresRegex, '-'); if (test(leadingDashesRegex, key)) { - return has(this, key) || - has(v8Flags, - replace( - replace( - key, - leadingDashesRegex, - '' - ), - replaceDashesRegex, - '_' - ) - ); + key = replace(key, trailingValuesRegex, ''); + return has(this, key); } - return has(nodeFlags, key) || - has(v8Flags, replace(key, replaceDashesRegex, '_')); + return has(nodeFlags, key); } return false; } @@ -862,7 +843,7 @@ return process.allowedNodeEnvironmentFlags = Object.freeze( new NodeEnvironmentFlagsSet( - allowedNodeEnvironmentFlags.concat(allowedV8EnvironmentFlags) + allowedNodeEnvironmentFlags )); }; diff --git a/src/node_options-inl.h b/src/node_options-inl.h index 1d1eda5475..277121036e 100644 --- a/src/node_options-inl.h +++ b/src/node_options-inl.h @@ -307,6 +307,12 @@ void OptionsParser<Options>::Parse( if (equals_index != std::string::npos) original_name += '='; + // Normalize by replacing `_` with `-` in options. + for (std::string::size_type i = 2; i < name.size(); ++i) { + if (name[i] == '_') + name[i] = '-'; + } + { auto it = aliases_.end(); // Expand aliases: @@ -341,19 +347,6 @@ void OptionsParser<Options>::Parse( auto it = options_.find(name); - if (it == options_.end()) { - // We would assume that this is a V8 option if neither we nor any child - // parser knows about it, so we convert - to _ for - // canonicalization (since V8 accepts both) and look up again in order - // to find a match. - // TODO(addaleax): Make the canonicalization unconditional, i.e. allow - // both - and _ in Node's own options as well. - std::string::size_type index = 2; // Start after initial '--'. - while ((index = name.find('-', index + 1)) != std::string::npos) - name[index] = '_'; - it = options_.find(name); - } - if ((it == options_.end() || it->second.env_setting == kDisallowedInEnvironment) && required_env_settings == kAllowedInEnvironment) { diff --git a/src/node_options.cc b/src/node_options.cc index 156ea11fe9..4951679994 100644 --- a/src/node_options.cc +++ b/src/node_options.cc @@ -102,8 +102,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() { &EnvironmentOptions::experimental_worker, kAllowedInEnvironment); AddOption("--expose-internals", "", &EnvironmentOptions::expose_internals); - // TODO(addaleax): Remove this when adding -/_ canonicalization to the parser. - AddAlias("--expose_internals", "--expose-internals"); AddOption("--loader", "(with --experimental-modules) use the specified file as a " "custom loader", @@ -204,15 +202,15 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() { kAllowedInEnvironment); // Explicitly add some V8 flags to mark them as allowed in NODE_OPTIONS. - AddOption("--abort_on_uncaught_exception", + AddOption("--abort-on-uncaught-exception", "aborting instead of exiting causes a core file to be generated " "for analysis", V8Option{}, kAllowedInEnvironment); - AddOption("--max_old_space_size", "", V8Option{}, kAllowedInEnvironment); - AddOption("--perf_basic_prof", "", V8Option{}, kAllowedInEnvironment); - AddOption("--perf_prof", "", V8Option{}, kAllowedInEnvironment); - AddOption("--stack_trace_limit", "", V8Option{}, kAllowedInEnvironment); + AddOption("--max-old-space-size", "", V8Option{}, kAllowedInEnvironment); + AddOption("--perf-basic-prof", "", V8Option{}, kAllowedInEnvironment); + AddOption("--perf-prof", "", V8Option{}, kAllowedInEnvironment); + AddOption("--stack-trace-limit", "", V8Option{}, kAllowedInEnvironment); Insert(&EnvironmentOptionsParser::instance, &PerIsolateOptions::get_per_env_options); diff --git a/test/parallel/test-cli-node-options-disallowed.js b/test/parallel/test-cli-node-options-disallowed.js index 0351f83c52..733e5cd7c7 100644 --- a/test/parallel/test-cli-node-options-disallowed.js +++ b/test/parallel/test-cli-node-options-disallowed.js @@ -29,7 +29,6 @@ disallow('--interactive'); disallow('-i'); disallow('--v8-options'); disallow('--'); -disallow('--no_warnings'); // Node options don't allow '_' instead of '-'. function disallow(opt) { const env = Object.assign({}, process.env, { NODE_OPTIONS: opt }); diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js index c7096a3acb..4ff63009a7 100644 --- a/test/parallel/test-cli-node-options.js +++ b/test/parallel/test-cli-node-options.js @@ -19,6 +19,7 @@ expect(`-r ${printA}`, 'A\nB\n'); expect(`-r ${printA} -r ${printA}`, 'A\nB\n'); expect('--no-deprecation', 'B\n'); expect('--no-warnings', 'B\n'); +expect('--no_warnings', 'B\n'); expect('--trace-warnings', 'B\n'); expect('--redirect-warnings=_', 'B\n'); expect('--trace-deprecation', 'B\n'); diff --git a/test/parallel/test-pending-deprecation.js b/test/parallel/test-pending-deprecation.js index f8b4ec8e5b..bb11d84bcc 100644 --- a/test/parallel/test-pending-deprecation.js +++ b/test/parallel/test-pending-deprecation.js @@ -36,6 +36,14 @@ switch (process.argv[2]) { assert.strictEqual(code, 0, message('--pending-deprecation')); })); + // Test the --pending_deprecation command line switch. + fork(__filename, ['switch'], { + execArgv: ['--pending_deprecation'], + silent: true + }).on('exit', common.mustCall((code) => { + assert.strictEqual(code, 0, message('--pending_deprecation')); + })); + // Test the NODE_PENDING_DEPRECATION environment var. fork(__filename, ['env'], { env: Object.assign({}, process.env, { NODE_PENDING_DEPRECATION: 1 }), diff --git a/test/parallel/test-process-env-allowed-flags.js b/test/parallel/test-process-env-allowed-flags.js index b853bdd539..ddd8d894ea 100644 --- a/test/parallel/test-process-env-allowed-flags.js +++ b/test/parallel/test-process-env-allowed-flags.js @@ -19,10 +19,10 @@ require('../common'); ].concat(process.config.variables.v8_enable_inspector ? [ '--inspect-brk', 'inspect-brk', + '--inspect_brk', ] : []); const badFlags = [ - '--inspect_brk', 'INSPECT-BRK', '--INSPECT-BRK', '--r', |