-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Adds additional validation used primarily in brackets-registry #3384
Conversation
dangoor
commented
Apr 9, 2013
- confirm that brackets.engine is a valid semver range
- normalizes author and contributors to always be objects
- allows rejection based on a word list
author and contributors fields are normalized to objects per the description here: https://npmjs.org/doc/json.html#people-fields-author-contributors
… description fields. Also adds the new validation strings.
to @njx |
var result = { | ||
name: parts[1] | ||
}; | ||
if (parts[2]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we were in optimize-the-heck-out-of-our-JS land I would suggest getting rid of the if
s and just putting email: parts[2], url: parts[3]
in the constructed object, since by definition they'd be undefined anyway if there was no match. But the ex-non-JS guy in me likes the clarity :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought of that, but I didn't like how the resulting object actually had email and url keys even if those were undefined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that shows my JS ignorance. I just assumed it wouldn't set the key. Learn something new every day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually had to fire up node this morning to confirm that this is how it works...
> foo = { name: "Hi there", email: undefined }
{ name: 'Hi there', email: undefined }
> Object.keys(foo)
[ 'name', 'email' ]
Looks good. I'm looking forward to us checking a list of naughty words into the Brackets codebase. |
Adds additional validation used primarily in brackets-registry
Feel free to push a new commit to master directly with the comment fixes. |
OK, will update the comments. I thought JSDoc was supposed to grab the param name from the function arguments, but you're right that it doesn't. I'm actually not planning to check naughty words in. I was thinking of just making disallowed words a config option in brackets-registry :) |
@@ -317,6 +317,8 @@ define({ | |||
"BAD_PACKAGE_NAME" : "{0} is an invalid package name.", | |||
"MISSING_PACKAGE_VERSION" : "The package.json file doesn't specify a package version.", | |||
"INVALID_VERSION_NUMBER" : "The package version number ({0}) is invalid.", | |||
"INVALID_BRACKETS_VERSION" : "The Brackets compatibility string {{0}} is invalid.", | |||
"DISALLOWED_WORDS" : "The words {{1}} are not allowed in the {{1}} field.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dangoor Maybe this should be The words {{0}} are not allowed in the {{1}} field. ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it's probably the other way around :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, Chema. Thanks! (and yes, it should be {{1}} and {{0}}).