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

Already on GitHub? Sign in to your account

Checking for NaN #14

Merged
merged 2 commits into from Jan 9, 2014

Conversation

Projects
None yet
3 participants
Owner

dominicbarnes commented Jan 7, 2014

Addresses issue #12 by adding a branch to check for NaN. (which returns "nan")

@yorkie yorkie and 1 other commented on an outdated diff Jan 7, 2014

@@ -22,7 +22,21 @@ module.exports = function(val){
if (val === null) return 'null';
if (val === undefined) return 'undefined';
+ if (isNaN(val)) return 'nan';
@yorkie

yorkie Jan 7, 2014

Contributor

hi, @dominicbarnes
maybe returns 'NaN' is better? it's a more friendly show.

@dominicbarnes

dominicbarnes Jan 7, 2014

Owner

I figured all lowercase was keeping the same convention, but I'm open to changing it.

@dominicbarnes dominicbarnes added a commit that referenced this pull request Jan 9, 2014

@dominicbarnes dominicbarnes Merge pull request #14 from component/NaN
Checking for NaN, fixes #12
5722c0d

@dominicbarnes dominicbarnes merged commit 5722c0d into master Jan 9, 2014

@dominicbarnes dominicbarnes deleted the NaN branch Jan 9, 2014

Contributor

ianstormtaylor commented Jan 13, 2014

didn't realize this earlier, but 'NaN' should probably be 'nan' because all of the other types are all lowercase

Owner

dominicbarnes commented Jan 13, 2014

I was going back and forth on this one, but I think I agree with @ianstormtaylor. I'll change it now before too many people depend on it.

Owner

dominicbarnes commented Jan 13, 2014

Changed via c817cb8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment