Skip to content
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.

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

fixed some functions might fail if not given a string #121

Closed
wants to merge 1 commit into from

Conversation

Andreas-Hjortland
Copy link

isDigit and one of the ts.addParser.is would fail if you did not give it a string,
so I just added s.toString() when doing the regex matches.

isDigit and ts.addParser.is would fail if you did not give it a string,
so I just added s.toString() when doing the regex matches.
@thezoggy
Copy link
Collaborator

can you resubmit your changes without modifying the line ending? as it shows that you modified every line.. thus making it really hard to tell what you even changed..

@Andreas-Hjortland
Copy link
Author

I sorry. I will do it right away

@Mottie
Copy link
Owner

Mottie commented Aug 16, 2012

It looks like you just added a toString() here:

this.isDigit = function(s) {
    // replace all unwanted chars and match.
    return (/^[\-+(]?\d+[)]?$/).test(s.toString().replace(/[,.'\s]/g, ''));
};

Please don't bother with a pull request. I had a hard time merging the last one into my current working code (I still suck at git LOL)... I hope I'll get v2.4 out there soon, with this change of course! ;)

@Andreas-Hjortland
Copy link
Author

It was just that. I suck at git myself as I almost have no experience with it. Good luck with further development of the library ^^

Fra: Rob G [mailto:notifications@github.com]
Sendt: 16 August 2012 17:18
Til: Mottie/tablesorter
Kopi: Andreas Hjortland
Emne: Re: [tablesorter] fixed some functions might fail if not given a string (#121)

It looks like you just added a toString() here:

this.isDigit = function(s) {
// replace all unwanted chars and match.
return (/^[-+(]?\d+[)]?$/).test(s.toString().replace(/[,.'\s]/g, ''));
};

Please don't bother with a pull request. I had a hard time merging the last one into my current working code (I still suck at git LOL)... I hope I'll get v2.4 out there soon, with this change of course! ;)


Reply to this email directly or view it on GitHub #121 (comment) .

https://github.com/notifications/beacon/rBvDsDai0B5X7hPgvVK0Rif2Sfh6jcLyKS_KbCBW8Gl34CKFiwwPNQR5c4nXpuEv.gif

@Mottie
Copy link
Owner

Mottie commented Aug 19, 2012

I just realized I did modify the isDigit function to prevent problems with non-strings:

ts.isDigit = function(s) {
    // replace all unwanted chars and match
    return isNaN(s) ? (/^[\-+(]?\d+[)]?$/).test(s.replace(/[,.'\s]/g, '')) : true;
};

so we probably don't need the s.toString(), but I think I'll go ahead and add it just in case :P

@Andreas-Hjortland
Copy link
Author

isNaN evaluates to true for undefined, which doesn’t have a replace function, so I still think a toString() will be applicable. I also noticed that this function will return true if you give it a Boolean value, because isNaN(true) and isNaN(false) evaluates to false. Which then returns true ^^

Fra: Rob G [mailto:notifications@github.com]
Sendt: 19 August 2012 06:54
Til: Mottie/tablesorter
Kopi: Andreas Hjortland
Emne: Re: [tablesorter] fixed some functions might fail if not given a string (#121)

I just realized I did modify the isDigit function to prevent problems with non-strings:

ts.isDigit = function(s) {
// replace all unwanted chars and match
return isNaN(s) ? (/^[-+(]?\d+[)]?$/).test(s.replace(/[,.'\s]/g, '')) : true;
};

so we probably don't need the s.toString(), but I think I'll go ahead and add it just in case :P


Reply to this email directly or view it on GitHub #121 (comment) .

https://github.com/notifications/beacon/rBvDsDai0B5X7hPgvVK0Rif2Sfh6jcLyKS_KbCBW8Gl34CKFiwwPNQR5c4nXpuEv.gif

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

Successfully merging this pull request may close these issues.

3 participants