Skip to content

Commit

Permalink
fix: feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
Eomm committed Aug 20, 2022
1 parent df0c4fb commit eae0933
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 11 deletions.
11 changes: 5 additions & 6 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const {
} = require('./internal/validators');

const {
kEmptyObject
kEmptyObject,
} = require('./internal/util');

const {
Expand Down Expand Up @@ -154,10 +154,9 @@ function storeOption(longOption, optionValue, options, values) {
* | boolean
* | string[]
* | boolean[]} optionValue - default value from option config
* @param {object} options - option configs, from parseArgs({ options })
* @param {object} values - option values returned in `values` by parseArgs
*/
function storeDefaultOption(longOption, optionValue, options, values) {
function storeDefaultOption(longOption, optionValue, values) {
if (longOption === '__proto__') {
return; // No. Just no.
}
Expand Down Expand Up @@ -376,16 +375,16 @@ const parseArgs = (config = kEmptyObject) => {

// Phase 3: fill in default values for missing args
const defaultValueOptions = ArrayPrototypeFilter(
ObjectEntries(options), (option) => {
return useDefaultValueOption(option, result.values);
ObjectEntries(options), ({ 0: longOption,
1: optionConfig }) => {
return useDefaultValueOption(longOption, optionConfig, result.values);
});

if (defaultValueOptions.length > 0) {
ArrayPrototypeForEach(defaultValueOptions, ({ 0: longOption,
1: optionConfig }) => {
storeDefaultOption(longOption,
optionConfig.defaultValue,
options,
result.values);
});

Expand Down
5 changes: 5 additions & 0 deletions internal/validators.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
'use strict';

// This file is a proxy of the original file located at:
// https://github.com/nodejs/node/blob/main/lib/internal/validators.js
// Every addition or modification to this file must be evaluated
// during the PR review.

const {
ArrayIsArray,
ArrayPrototypeIncludes,
Expand Down
2 changes: 1 addition & 1 deletion test/default-values.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ test('proto as default value must be ignored', (t) => {
});


test('multiple as false should not expect an array', (t) => {
test('multiple as false should expect a String and not an array', (t) => {
const args = [];
const options = { alpha: { type: 'string', multiple: false, defaultValue: 42 } };
t.throws(() => {
Expand Down
8 changes: 4 additions & 4 deletions utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,12 @@ function findLongOptionForShort(shortOption, options) {
* Check if the given option includes a default value
* and that option has not been set by the input args.
*
* @param {array} options - option configs entry, from parseArgs({ options })
* @param {string} longOption - long option name e.g. 'foo'
* @param {object} optionConfig - the option configuration properties
* @param {object} values - option values returned in `values` by parseArgs
*/
function useDefaultValueOption({ 0: longOption,
1: optionConfig }, values) {
return optionConfig.defaultValue !== undefined &&
function useDefaultValueOption(longOption, optionConfig, values) {
return objectGetOwn(optionConfig, 'defaultValue') !== undefined &&
values[longOption] === undefined;
}

Expand Down

0 comments on commit eae0933

Please sign in to comment.