refactor: most recent code review changes (#118)

pkgjs · May 16, 2022 · 8943312 · 8943312
1 parent 6ca85cd
commit 8943312
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 34 deletions.
diff --git a/README.md b/README.md
@@ -21,7 +21,6 @@ added: REPLACEME
     Keys of `options` are the long names of options and values are an
     {Object} accepting the following properties:
     * `type` {string} Type of argument, which must be either `boolean` or `string`.
-      **Default:** `boolean`.
     * `multiple` {boolean} Whether this option can be provided multiple
       times. If `true`, all values will be collected in an array. If
       `false`, values for the option are last-wins. **Default:** `false`.
@@ -30,20 +29,21 @@ added: REPLACEME
     are encountered, or when arguments are passed that do not match the
     `type` configured in `options`.
     **Default:** `true`.
-  * `allowPositionals`: {boolean} Whether this command accepts positional arguments.
+  * `allowPositionals`: {boolean} Whether this command accepts positional
+    arguments.
     **Default:** `false` if `strict` is `true`, otherwise `true`.
 
-* Returns: {Object} An {Object} representing the parsed command line
-  arguments:
-  * `values` {Object} With properties and {string} or {boolean} values
-    corresponding to parsed options passed.
-  * `positionals` {string\[]}, containing positional arguments.
+* Returns: {Object} The parsed command line arguments:
+  * `values` {Object} A mapping of parsed option names with their {string}
+    or {boolean} values.
+  * `positionals` {string\[]} Positional arguments.
 
 Provides a higher level API for command-line argument parsing than interacting
-with `process.argv` directly. Takes a specification for the expected arguments and returns a structured object with the parsed options and positionals.
+with `process.argv` directly. Takes a specification for the expected arguments
+and returns a structured object with the parsed options and positionals.
 
 ```mjs
-import { parseArgs } from 'util';
+import { parseArgs } from 'node:util';
 const args = ['-f', '--bar', 'b'];
 const options = {
   foo: {
@@ -58,10 +58,12 @@ const {
   values,
   positionals
 } = parseArgs({ args, options });
+console.log(values, positionals);
+// Prints: [Object: null prototype] { foo: true, bar: 'b' } []
 ```
 
 ```cjs
-const { parseArgs } = require('util');
+const { parseArgs } = require('node:util');
 const args = ['-f', '--bar', 'b'];
 const options = {
   foo: {
@@ -76,6 +78,8 @@ const {
   values,
   positionals
 } = parseArgs({ args, options });
+console.log(values, positionals);
+// Prints: [Object: null prototype] { foo: true, bar: 'b' } []ss
 ```
 
 `util.parseArgs` is experimental and behavior may change. Join the

diff --git a/index.js b/index.js
@@ -1,15 +1,16 @@
 'use strict';
 
 const {
-  ArrayPrototypeConcat,
   ArrayPrototypeForEach,
+  ArrayPrototypeIncludes,
   ArrayPrototypeShift,
   ArrayPrototypeSlice,
   ArrayPrototypePush,
+  ArrayPrototypePushApply,
+  ArrayPrototypeUnshiftApply,
   ObjectEntries,
   ObjectPrototypeHasOwnProperty: ObjectHasOwn,
   StringPrototypeCharAt,
-  StringPrototypeIncludes,
   StringPrototypeIndexOf,
   StringPrototypeSlice,
 } = require('./primordials');
@@ -32,7 +33,7 @@ const {
   isShortOptionAndValue,
   isShortOptionGroup,
   objectGetOwn,
-  optionsGetOwn
+  optionsGetOwn,
 } = require('./utils');
 
 const {
@@ -48,29 +49,29 @@ function getMainArgs() {
   // This function is a placeholder for proposed process.mainArgs.
   // Work out where to slice process.argv for user supplied arguments.
 
-  // Electron is an interested example, with work-arounds implemented in
+  // Electron is an interesting example, with workarounds implemented in
   // Commander and Yargs. Hopefully Electron would support process.mainArgs
-  // itself and render this work-around moot.
+  // itself and render this workaround moot.
   //
   // In a bundled Electron app, the user CLI args directly
   // follow executable. (No special processing required for unbundled.)
   // 1) process.versions.electron is either set by electron, or undefined
-  //    see https://github.com/electron/electron/blob/master/docs/api/process.md#processversionselectron-readonly
+  //    see: https://www.electronjs.org/docs/latest/api/process#processversionselectron-readonly
   // 2) process.defaultApp is undefined in a bundled Electron app, and set
   //    in an unbundled Electron app
-  //    see https://github.com/electron/electron/blob/master/docs/api/process.md#processversionselectron-readonly
+  //    see: https://www.electronjs.org/docs/latest/api/process#processdefaultapp-readonly
   // (Not included in tests as hopefully temporary example.)
   /* c8 ignore next 3 */
-  if (process.versions && process.versions.electron && !process.defaultApp) {
+  if (process.versions?.electron && !process.defaultApp) {
     return ArrayPrototypeSlice(process.argv, 1);
   }
 
   // Check node options for scenarios where user CLI args follow executable.
   const execArgv = process.execArgv;
-  if (StringPrototypeIncludes(execArgv, '-e') ||
-      StringPrototypeIncludes(execArgv, '--eval') ||
-      StringPrototypeIncludes(execArgv, '-p') ||
-      StringPrototypeIncludes(execArgv, '--print')) {
+  if (ArrayPrototypeIncludes(execArgv, '-e') ||
+      ArrayPrototypeIncludes(execArgv, '--eval') ||
+      ArrayPrototypeIncludes(execArgv, '-p') ||
+      ArrayPrototypeIncludes(execArgv, '--print')) {
     return ArrayPrototypeSlice(process.argv, 1);
   }
 
@@ -107,6 +108,7 @@ To specify an option argument starting with a dash use ${example}.`;
  * @param {object} options - option configs, from parseArgs({ options })
  * @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long`
  * @param {boolean} strict - show errors, from parseArgs({ strict })
+ * @param {boolean} allowPositionals - from parseArgs({ allowPositionals })
  */
 function checkOptionUsage(longOption, optionValue, options,
                           shortOrLong, strict, allowPositionals) {
@@ -203,7 +205,7 @@ const parseArgs = (config = { __proto__: null }) => {
     positionals: []
   };
 
-  let remainingArgs = ArrayPrototypeSlice(args);
+  const remainingArgs = ArrayPrototypeSlice(args);
   while (remainingArgs.length > 0) {
     const arg = ArrayPrototypeShift(remainingArgs);
     const nextArg = remainingArgs[0];
@@ -216,7 +218,7 @@ const parseArgs = (config = { __proto__: null }) => {
       }
 
       // Everything after a bare '--' is considered a positional argument.
-      result.positionals = ArrayPrototypeConcat(
+      ArrayPrototypePushApply(
         result.positionals,
         remainingArgs
       );
@@ -257,7 +259,7 @@ const parseArgs = (config = { __proto__: null }) => {
           break; // finished short group
         }
       }
-      remainingArgs = ArrayPrototypeConcat(expanded, remainingArgs);
+      ArrayPrototypeUnshiftApply(remainingArgs, expanded);
       continue;
     }
 
@@ -309,5 +311,5 @@ const parseArgs = (config = { __proto__: null }) => {
 };
 
 module.exports = {
-  parseArgs
+  parseArgs,
 };
diff --git a/utils.js b/utils.js
@@ -6,12 +6,11 @@ const {
   ObjectPrototypeHasOwnProperty: ObjectHasOwn,
   StringPrototypeCharAt,
   StringPrototypeIncludes,
-  StringPrototypeSlice,
   StringPrototypeStartsWith,
 } = require('./primordials');
 
 const {
-  validateObject
+  validateObject,
 } = require('./validators');
 
 // These are internal utilities to make the parsing logic easier to read, and
@@ -85,7 +84,7 @@ function isLoneShortOption(arg) {
 function isLoneLongOption(arg) {
   return arg.length > 2 &&
     StringPrototypeStartsWith(arg, '--') &&
-    !StringPrototypeIncludes(StringPrototypeSlice(arg, 3), '=');
+    !StringPrototypeIncludes(arg, '=', 3);
 }
 
 /**
@@ -97,7 +96,7 @@ function isLoneLongOption(arg) {
 function isLongOptionAndValue(arg) {
   return arg.length > 2 &&
     StringPrototypeStartsWith(arg, '--') &&
-    StringPrototypeIncludes(StringPrototypeSlice(arg, 3), '=');
+    StringPrototypeIncludes(arg, '=', 3);
 }
 
 /**
@@ -164,11 +163,11 @@ function isShortOptionAndValue(arg, options) {
  */
 function findLongOptionForShort(shortOption, options) {
   validateObject(options, 'options');
-  const { 0: longOption } = ArrayPrototypeFind(
+  const longOptionEntry = ArrayPrototypeFind(
     ObjectEntries(options),
     ({ 1: optionConfig }) => objectGetOwn(optionConfig, 'short') === shortOption
-  ) || [];
-  return longOption || shortOption;
+  );
+  return longOptionEntry?.[0] ?? shortOption;
 }
 
 module.exports = {
@@ -181,5 +180,5 @@ module.exports = {
   isShortOptionAndValue,
   isShortOptionGroup,
   objectGetOwn,
-  optionsGetOwn
+  optionsGetOwn,
 };