Skip to content

url.parse and debug #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wraithgar wants to merge 3 commits into
base: main
Choose a base branch
from
+95 −143
Open

url.parse and debug #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1dd54d6
feat: stop using url.parse
wraithgar Apr 6, 2026
ffa40d6
feat: remove debug statements
wraithgar Apr 6, 2026
ea949b4
fix: inline comments
wraithgar Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions lib/debug.js
View file
Open in desktop

This file was deleted.

50 changes: 7 additions & 43 deletions lib/nopt-lib.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const abbrev = require('abbrev')
const debug = require('./debug')
const defaultTypeDefs = require('./type-defs')

const hasOwn = (o, k) => Object.prototype.hasOwnProperty.call(o, k)
Expand Down Expand Up @@ -31,8 +30,6 @@ function nopt (args, {
typeDefault,
dynamicTypes,
} = {}) {
debug(types, shorthands, args, typeDefs)

const data = {}
const argv = {
remain: [],
Expand Down Expand Up @@ -89,7 +86,6 @@ function clean (data, {
return
}
let val = data[k]
debug('val=%j', val)
const isArray = Array.isArray(val)
let [hasType, rawType] = getType(k, { types, dynamicTypes })
let type = rawType
Expand All @@ -106,25 +102,19 @@ function clean (data, {
type = [type]
}

debug('val=%j', val)
debug('types=', type)
val = val.map((v) => {
// if it's an unknown value, then parse false/true/null/numbers/dates
if (typeof v === 'string') {
debug('string %j', v)
v = v.trim()
if ((v === 'null' && ~type.indexOf(null))
|| (v === 'true' &&
(~type.indexOf(true) || hasTypeDef(type, BooleanType)))
|| (v === 'false' &&
(~type.indexOf(false) || hasTypeDef(type, BooleanType)))) {
v = JSON.parse(v)
debug('jsonable %j', v)
} else if (hasTypeDef(type, NumberType) && !isNaN(v)) {
debug('convert to number', v)
v = +v
} else if (hasTypeDef(type, DateType) && !isNaN(Date.parse(v))) {
debug('convert to date', v)
v = new Date(v)
}
}
Expand All @@ -148,32 +138,24 @@ function clean (data, {

const d = {}
d[k] = v
debug('prevalidated val', d, v, rawType)
if (!validate(d, k, v, rawType, { typeDefs })) {
if (invalidHandler) {
invalidHandler(k, v, rawType, data)
} else if (invalidHandler !== false) {
debug('invalid: ' + k + '=' + v, rawType)
}
return remove
}
debug('validated v', d, v, rawType)
return d[k]
}).filter((v) => v !== remove)

// if we allow Array specifically, then an empty array is how we
// express 'no value here', not null. Allow it.
if (!val.length && doesNotHaveTypeDef(type, ArrayType)) {
debug('VAL HAS NO LENGTH, DELETE IT', val, k, type.indexOf(ArrayType))
delete data[k]
} else if (isArray) {
debug(isArray, data[k], val)
data[k] = val
} else {
data[k] = val[0]
}

debug('k=%s val=%j', k, val, data[k])
})
}

Expand All @@ -199,21 +181,21 @@ function validate (data, k, val, type, { typeDefs } = {}) {
}

// Original comment:
// NaN is poisonous. Means that something is not allowed.
// New comment: Changing this to an isNaN check breaks a lot of tests.
// Something is being assumed here that is not actually what happens in
// practice. Fixing it is outside the scope of getting linting to pass in
// this repo. Leaving as-is for now.
// NaN is poisonous, it means that something is not allowed.

// New comment:
// Changing this to an isNaN check breaks a lot of tests.
// Something is being assumed here that is not actually what happens in practice.
// Fixing it is outside the scope of getting linting to pass in this repo.
// Leaving as-is for now.
/* eslint-disable-next-line no-self-compare */
if (type !== type) {
debug('Poison NaN', k, val, type)
delete data[k]
return false
}

// explicit list of values
if (val === type) {
debug('Explicitly allowed %j', val)
data[k] = val
return true
}
Expand All @@ -222,7 +204,6 @@ function validate (data, k, val, type, { typeDefs } = {}) {
let ok = false
const types = Object.keys(typeDefs)
for (let i = 0, l = types.length; i < l; i++) {
debug('test type %j %j %j', k, val, types[i])
const t = typeDefs[types[i]]
if (t && (
(type && type.name && t.type && t.type.name) ?
Expand All @@ -238,7 +219,6 @@ function validate (data, k, val, type, { typeDefs } = {}) {
}
}
}
debug('OK? %j (%j %j %j)', ok, k, val, types[types.length - 1])

if (!ok) {
delete data[k]
Expand All @@ -258,16 +238,11 @@ function parse (args, data, remain, {
const NumberType = typeDefs.Number?.type
const ArrayType = typeDefs.Array?.type
const BooleanType = typeDefs.Boolean?.type

debug('parse', args, data, remain)

const abbrevs = abbrev(Object.keys(types))
debug('abbrevs=%j', abbrevs)
const shortAbbr = abbrev(Object.keys(shorthands))

for (let i = 0; i < args.length; i++) {
let arg = args[i]
debug('arg', arg)

if (arg.match(/^-{2,}$/)) {
// done with keys.
Expand All @@ -289,7 +264,6 @@ function parse (args, data, remain, {
// see if it's a shorthand
// if so, splice and back up to re-parse it.
const shRes = resolveShort(arg, shortAbbr, abbrevs, { shorthands, abbrevHandler })
debug('arg=%j shRes=%j', arg, shRes)
if (shRes) {
args.splice.apply(args, [i, 1].concat(shRes))
if (arg !== shRes[0]) {
Expand All @@ -308,8 +282,6 @@ function parse (args, data, remain, {
if (abbrevs[arg] && abbrevs[arg] !== arg) {
if (abbrevHandler) {
abbrevHandler(arg, abbrevs[arg])
} else if (abbrevHandler !== false) {
debug(`abbrev: ${arg} -> ${abbrevs[arg]}`)
}
arg = abbrevs[arg]
}
Expand Down Expand Up @@ -352,11 +324,6 @@ function parse (args, data, remain, {
} else {
unknownHandler(arg)
}
} else if (unknownHandler !== false) {
debug(`unknown: ${arg}`)
if (hangingLa) {
debug(`unknown: ${la} parsed as normal opt`)
}
}
}

Expand Down Expand Up @@ -442,7 +409,6 @@ const singleCharacters = (arg, shorthands) => {
return l
}, {})
shorthands[SINGLES] = singles
debug('shorthand singles', singles)
}
const chrs = arg.split('').filter((c) => singles[c])
return chrs.join('') === arg ? chrs : null
Expand Down Expand Up @@ -490,8 +456,6 @@ function resolveShort (arg, ...rest) {
if (shortAbbr[arg]) {
if (abbrevHandler) {
abbrevHandler(arg, shortAbbr[arg])
} else if (abbrevHandler !== false) {
debug(`abbrev: ${arg} -> ${shortAbbr[arg]}`)
}
arg = shortAbbr[arg]
}
Expand Down
6 changes: 2 additions & 4 deletions lib/nopt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
const lib = require('./nopt-lib')
const defaultTypeDefs = require('./type-defs')

// This is the version of nopt's API that requires setting typeDefs and invalidHandler
// on the required `nopt` object since it is a singleton. To not do a breaking change
// an API that requires all options be passed in is located in `nopt-lib.js` and
// exported here as lib.
// This is the version of nopt's API that requires setting typeDefs and invalidHandler on the required `nopt` object since it is a singleton.
// To not do a breaking change an API that requires all options be passed in is located in `nopt-lib.js` and exported here as lib.
// TODO(breaking): make API only work in non-singleton mode

module.exports = exports = nopt
Expand Down
21 changes: 8 additions & 13 deletions lib/type-defs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const url = require('url')
const path = require('path')
const Stream = require('stream').Stream
const os = require('os')
const debug = require('./debug')
const os = require('node:os')
const path = require('node:path')
const { Stream } = require('node:stream')
const { URL } = require('node:url')

function validateString (data, k, val) {
data[k] = String(val)
Expand Down Expand Up @@ -31,7 +30,6 @@ function validatePath (data, k, val) {
}

function validateNumber (data, k, val) {
debug('validate Number %j %j %j', k, val, isNaN(val))
if (isNaN(val)) {
return false
}
Expand All @@ -40,7 +38,6 @@ function validateNumber (data, k, val) {

function validateDate (data, k, val) {
const s = Date.parse(val)
debug('validate Date %j %j %j', k, val, s)
if (isNaN(s)) {
return false
}
Expand All @@ -63,13 +60,11 @@ function validateBoolean (data, k, val) {
}

function validateUrl (data, k, val) {
// Changing this would be a breaking change in the npm cli
/* eslint-disable-next-line node/no-deprecated-api */
val = url.parse(String(val))
if (!val.host) {
const parsed = URL.parse(String(val))
if (!parsed) {
return false
}
data[k] = val.href
data[k] = parsed.href
}

function validateStream (data, k, val) {
Expand All @@ -82,7 +77,7 @@ function validateStream (data, k, val) {
module.exports = {
String: { type: String, validate: validateString },
Boolean: { type: Boolean, validate: validateBoolean },
url: { type: url, validate: validateUrl },
url: { type: URL, validate: validateUrl },
Number: { type: Number, validate: validateNumber },
path: { type: path, validate: validatePath },
Stream: { type: Stream, validate: validateStream },
Expand Down
Loading
Loading